Review of "Blackstrat Desktop Icons" version 5

Details Page Preview

Free-placement desktop icons with context menu. This extension accesses the system clipboard to support cut, copy, and paste of files between the desktop and other applications. Known limitation: cut/paste and drag&drop of files between the desktop and Nautilus (Files) are not fully supported yet.

Extension Homepage
https://github.com/BlackStrat4/blackstrat-desktopicons

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 6 manual_review 1

Shexli found 7 issues that may need reviewer attention.

EGO-X-004 warning

extensions should avoid synchronous file IO in shell code

Shell code should avoid synchronous file IO APIs like `GLib.file_get_contents()` and `Gio.File.load_contents()`.

File Operations

  • extension.js:1723
    file.load_contents(null)

EGO-A-005 manual_review

extensions should not access the clipboard directly

Direct clipboard access via `St.Clipboard.get_default()` requires reviewer scrutiny.

Review Guidelines

  • extension.js:1133
    St.Clipboard.get_default()
  • extension.js:1223
    St.Clipboard.get_default()
  • extension.js:2465
    St.Clipboard.get_default()
  • extension.js:2472
    St.Clipboard.get_default()
  • extension.js:2479
    St.Clipboard.get_default()
  • extension.js:2504
    St.Clipboard.get_default()
  • extension.js:2510
    St.Clipboard.get_default()

EGO-P-006 warning

unnecessary build and translation artifacts should not be shipped

Compiled GSettings schemas should not be shipped for 45+ packages.

Don't include unnecessary files

  • schemas/gschemas.compiled
    schemas/gschemas.compiled

EGO-L-002 warning

objects created by extension should be destroyed in disable()

Objects assigned in `enable()` are missing matching `.destroy()` calls in `disable()` or its helper methods.

Destroy all objects

  • extension.js:338
            this.selectionBox = new St.Widget({
                style: "background-color: rgba(53, 132, 228, 0.25); border: 1px solid #3584e4; border-radius: 2px;",
                visible: false
            })

EGO-L-005 warning

owned object references should be released in disable()

Owned references that are cleaned up in `disable()` should also be released with `null` or `undefined`.

Destroy all objects

  • extension.js:50
                this._thumbnailFactory = GnomeDesktop.DesktopThumbnailFactory.new(
                    GnomeDesktop.DesktopThumbnailSize.LARGE
                )
  • extension.js:338
            this.selectionBox = new St.Widget({
                style: "background-color: rgba(53, 132, 228, 0.25); border: 1px solid #3584e4; border-radius: 2px;",
                visible: false
            })

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • extension.js:155
            box.connect('notify::hover', () => {
                if (box.hover && !this._draggingAnyIcon && !this._activeRename) {
                    label.set_height(lineHeight * 3 + 4);
                    label.clutter_text.ellipsize = Pango.EllipsizeMode.NONE;
                } else {
                    label.set_hei
  • extension.js:347
            this.desktopLayer.connect('button-press-event', (actor, event) => {
                let button = event.get_button();
    
                if (button === 3) {
                    this._clearRightClickFocus();
                    this._showDesktopContextMenu(event);
                    return Clutter.EVENT_STOP;
        
  • extension.js:394
            this.desktopLayer.connect('button-release-event', (actor, event) => {
                if (this._isMarqueeSelecting && event.get_button() === 1) {
                    this._isMarqueeSelecting = false;
                    this.selectionBox.hide();
                    return Clutter.EVENT_STOP;
                }
     
  • extension.js:379
            this.desktopLayer.connect('motion-event', (actor, event) => {
                if (this._isMarqueeSelecting) {
                    let [cx, cy] = event.get_coords();
                    let x = Math.min(this._marqueeStartX, cx);
                    let y = Math.min(this._marqueeStartY, cy);
                    l

EGO-L-004 warning

main loop sources should be removed in disable()

Main loop sources assigned in `enable()` are missing matching removals in `disable()` or its helper methods.

Remove main loop sources

  • extension.js:1430
                GLib.idle_add(GLib.PRIORITY_DEFAULT, () => {
                    try {
                        let monitor = Main.layoutManager.primaryMonitor;
                        let [pw, ph] = panel.get_size();
                        let x = monitor.x + Math.max(0, Math.floor((monitor.width - pw) / 2));
            
  • extension.js:1623
                GLib.idle_add(GLib.PRIORITY_DEFAULT, () => {
                    try {
                        let monitor = Main.layoutManager.primaryMonitor;
                        let [pw, ph] = panel.get_size();
                        let x = monitor.x + Math.max(0, Math.floor((monitor.width - pw) / 2));
            
  • extension.js:2377
            GLib.idle_add(GLib.PRIORITY_DEFAULT, () => {
                selectRenameText();
    
                renameSelectionFallbackId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 100, () => {
                    renameSelectionFallbackId = 0;
                    if (!userTouchedRenameEntry && !renameSelectionDone) {
       
  • extension.js:672
                    GLib.timeout_add(GLib.PRIORITY_DEFAULT, 50, () => {
                        dummySource.destroy();
                        return GLib.SOURCE_REMOVE;
                    })

All Versions

Version Status
6 Unreviewed
5 Rejected
4 Rejected
3 Rejected
2 Rejected
1 Rejected

Previous Reviews on this Version

Dimitris auto- rejected
Auto-rejected because of new version 6 was uploaded