D24165: Add SettingsUtil utility class

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R856:64d6bab15fe6: Add SettingsUtils utility class (authored 
by broulik).

REPOSITORY
  R856 Plasma Browser Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24165?vs=66690=66750

REVISION DETAIL
  https://phabricator.kde.org/D24165

AFFECTED FILES
  extension/content-script.js
  extension/extension-utils.js
  extension/extension.js
  extension/manifest.json
  extension/options.html
  extension/options.js
  extension/utils.js

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Fabian Vogt]

fvogt accepted this revision.
fvogt added a comment.
This revision is now accepted and ready to land.


  Appears to work on ESR

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24165

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Carl Schwan]

ognarb added a comment.


  +1 but I couldn't test it (changed my distro recently and can't get pbi to 
work yet).
  
  The js class is available in firefox since Firefox 45 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#Browser_compatibility
 so it should work in firefox esr

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24165

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Kai Uwe Broulik]

broulik updated this revision to Diff 66690.
broulik edited the summary of this revision.
broulik added a comment.


  - Drop changed stuff for now
  - Catch exception directly in get and reject

REPOSITORY
  R856 Plasma Browser Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24165?vs=66684=66690

REVISION DETAIL
  https://phabricator.kde.org/D24165

AFFECTED FILES
  extension/content-script.js
  extension/extension-utils.js
  extension/extension.js
  extension/manifest.json
  extension/options.html
  extension/options.js
  extension/utils.js

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Kai Uwe Broulik]

broulik added a comment.


  In D24165#536531 , @fvogt wrote:
  
  > I can try if you'd like.
  
  
  That would be appreciated.

INLINE COMMENTS

> fvogt wrote in utils.js:19
> Wow, ES6 does indeed not support static members...

Yeah... :/

> fvogt wrote in utils.js:50
> So on FF nothing using `onChanged` will actually work?

Yes. The settings page actually sends a signal when the user changes something, 
which I might be able to catch... but this is for a later revision, I don't 
actually use the `onChanged` stuff just yet. The plan is mostly meant for some 
Schmankerl to have e.g. media controls turn on/off live, nothing essential.

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24165

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Fabian Vogt]

fvogt added a comment.


  > Probably needs some Firefox ESR 60 testing to see whether it can deal with 
the class stuff...
  
  I can try if you'd like.

INLINE COMMENTS

> options.js:137
>  });
>  // When the extension is reloaded, any call to extension APIs throws
>  } catch (e) {

This can be caught in SettingsUtils now

> utils.js:19
> +class SettingsUtils {
> +static storage() {
> +return (IS_FIREFOX ? chrome.storage.local : chrome.storage.sync);

Wow, ES6 does indeed not support static members...

> utils.js:50
> +static onChanged() {
> +// Firefox doesn't support change listeners, return a fake one
> +let changeListener = SettingsUtils.storage().onChanged;

So on FF nothing using `onChanged` will actually work?

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24165

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D24165: Add SettingsUtil utility class

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, fvogt, ognarb.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  For reading, writing, and eventually getting notified of changes to settings.
  Promise-based API taking into account defaults values.

TEST PLAN
  - Settings page loads and write settings fine
  - Content script loads settings fine
  - Extension script loads settings fine and sends them to the host
  
  Probably needs some Firefox ESR 60 testing to see whether it can deal with 
the `class` stuff...

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24165

AFFECTED FILES
  extension/content-script.js
  extension/extension-utils.js
  extension/extension.js
  extension/manifest.json
  extension/options.js
  extension/utils.js

To: broulik, #plasma, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart