1313155 - Remove dom/settings and related code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

B2G-related code gets removed from mozilla-central, see bug 1306391.

Comment 2

[Michelangelo De Simone [Offline] [:mds]]

Attachment: Bug 1313155 - Remove dom/settings and related code.

Review commit: https://reviewboard.mozilla.org/r/102384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102384/

Comment 3

[(no longer active)]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

https://reviewboard.mozilla.org/r/102384/#review103508

::: accessible/jsat/Utils.jsm
(Diff revision 1)
> -  let settings = Utils.win.navigator.mozSettings;
> -  if (!settings) {
> -    if (aOptions.callbackNow) {
> +  if (aOptions.callbackNow) {
> -      runCallback();
> +    runCallback();
> -    }
> +  }
> -    return;

You're changing the semantics of this function.  Right now we return after this block because |settings| is always null.  Now we're continuting to execute the rest of the function.

::: b2g/app/b2g.js
(Diff revision 1)
>  
> -// Enable TV Manager API
> -pref("dom.tv.enabled", true);
> -
> -// Enable Inputport Manager API
> -pref("dom.inputport.enabled", true);

Why these?

::: devtools/shared/discovery/discovery.js
(Diff revision 1)
>  
>  /**
>   * Manages the local device's name.  The name can be generated in serveral
>   * platform-specific ways (see |_generate|).  The aim is for each device on the
> - * same local network to have a unique name.  If the Settings API is available,
> + * same local network to have a unique name.
> - * the name is saved there to persist across reboots.

I don't dare review devtools code.  :-)  Can you get someone else to look into these parts?  jryans would probably be a good candidate.

::: dom/system/NetworkGeolocationProvider.js
(Diff revision 1)
>    listener: null,
>  
>    observe: function(aSubject, aTopic, aData) {
> -    if (aTopic != SETTINGS_CHANGED_TOPIC) {
> -      return;
> -    }

Looks like the whole observe method (and probably the respective addObserver call) can go away, no?

::: testing/profiles/prefs_general.js
(Diff revision 1)
>  // Enable Media Source Extensions for testing
>  user_pref("media.mediasource.mp4.enabled", true);
>  user_pref("media.mediasource.webm.enabled", true);
>  
> -// Enable mozContacts
> -user_pref("dom.mozContacts.enabled", true);

Why this?

Comment 4

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/1-2/

Comment 5

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

https://reviewboard.mozilla.org/r/102384/#review103508

> You're changing the semantics of this function.  Right now we return after this block because |settings| is always null.  Now we're continuting to execute the rest of the function.

As this function seems to have no caller I've removed it altogether. I don't think WinMouseScrollHandler is using that but I may be wrong.

> Why these?

I'm following up to your comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1309030#c4 by having good hygiene (inputport, TV and contacts are not on m-c anymore).

> Why this?

Ditto.

Comment 6

[Michelangelo De Simone [Offline] [:mds]]

:jryans would you please give us a feedback on this, as per comment #3?

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

https://reviewboard.mozilla.org/r/102384/#review104348

::: devtools/server/main.js
(Diff revision 2)
>          constructor: "ActorRegistryActor",
>          type: { global: true }
>        });
>      }
> -    if (Services.prefs.getBoolPref("dom.mozSettings.enabled")) {
> -      this.registerModule("devtools/server/actors/settings", {

Here are some additional file related to settings that should also be removed:

devtools/server/actor/settings.js
devtools/shared/specs/settings.js
devtools/shared/fronts/settings.js
devtools/client/webide/content/devicesettings.js
devtools/client/webide/content/devicesettings.xhtml

Also, these references to devicesettings in webide need updates:

http://searchfox.org/mozilla-central/search?q=devicesettings&case=true&regexp=false&path=

Comment 8

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/2-3/

Comment 9

[(no longer active)]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

https://reviewboard.mozilla.org/r/102384/#review104832

Thanks, looks great!

Comment 10

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/3-4/

Comment 11

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/4-5/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5566e83980ed
Remove dom/settings and related code. r=Ehsan

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for mass build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=68564288&repo=autoland&lineNumber=2654

Looks like at least one moz.build file still needs updated?

https://hg.mozilla.org/integration/autoland/rev/251ca9a04afc

Comment 14

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/5-6/

Comment 15

[Michelangelo De Simone [Offline] [:mds]]

(In reply to Wes Kocher (:KWierso) from comment #13)

> Looks like at least one moz.build file still needs updated?

Ouch, I'm so sorry for that.
I've updated it and pushed to try again.

Comment 16

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/6-7/

Comment 17

[Michelangelo De Simone [Offline] [:mds]]

:jryans I've been trying to pinpoint where this WebIDE failure [1] is coming from...
Do you have any feedback about that?

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=304f2977a15d&selectedJob=68603303

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Michelangelo De Simone [:mds] from comment #17)
> :jryans I've been trying to pinpoint where this WebIDE failure [1] is coming
> from...
> Do you have any feedback about that?

Looks like you need to additionally remove:

* Import of getSettingsFront[1]
* Code paths that reference settingsFront[2]

[1]: http://searchfox.org/mozilla-central/source/devtools/client/webide/modules/app-manager.js#20
[2]: http://searchfox.org/mozilla-central/search?q=settingsFront&case=true&path=

Comment 19

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8823917 [details]
Bug 1313155 - Remove dom/settings and related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102384/diff/7-8/

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b172f492f29f

Comment 21

[Michelangelo De Simone [Offline] [:mds]]

:Ehsan and :jryans: thank you very much for your help!

Comment 22

[Chris Mills [:cmills]]

I've archived all the B2G OS API stuff here (you'll see the settings interfaces listed):

https://developer.mozilla.org/en-US/docs/Archive/B2G_OS/API/

I've also added a note to the Fx53 release notes to make it clear that the API has been removed:

https://developer.mozilla.org/en-US/Firefox/Releases/53#DOM_HTML_DOM