Review of "Screen word translate" version 54

Details Page Preview

Translate word on the screen. Default web address is translate.google.com, you can add the web address for your own language. Also you can contribute your web address to my github repo. Use hotkey Ctrl+Alt+j to toggle the function. Use hotkey Ctrl+Alt+o to show popup window

Extension Homepage
https://github.com/sunwxg/gnome-shell-extension-dict

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) error 1 warning 6 manual_review 1

Shexli found 8 issues that may need reviewer attention.

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:131
    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-P-004 error

GSettings schema XML filename must match schema id

GSettings schema filename must match `<schema-id>.gschema.xml`.

GSettings Schemas

  • schemas/org.gnome.shell.extensions.flag.gschema.xml
    id='org.gnome.shell.extensions.dict' path='/org/gnome/shell/extensions/dict/'

EGO-P-007 warning

JavaScript files should be reachable from extension.js or prefs.js

Some JavaScript files are not reachable from `extension.js` or `prefs.js` imports.

Don't include unnecessary files

  • dict.js
  • history.js
  • store.js
  • util.js

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:106
            this.actor = new St.BoxLayout({ reactive: true,
                                        can_focus: true,
                                        track_hover: true})

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:106
            this.actor = new St.BoxLayout({ reactive: true,
                                        can_focus: true,
                                        track_hover: true})

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:466
    this._clickGesture.connect('recognize', () => { this._onButtonPress(); })

EGO-C45-001 warning

45+ preferences should use fillPreferencesWindow instead of getPreferencesWidget

45+ preferences code should use `fillPreferencesWindow()` instead of `getPreferencesWidget()`.

Preferences

  • prefs.js:386
        getPreferencesWidget() {
            let ui = new buildUi(this.getSettings());
            return ui.widget;
        }

All Versions

Previous Reviews on this Version

fmuellner active