1839948 - Intermittent TV /tests/toolkit/components/extensions/test/mochitest/test_ext_browserAction_getUserSettings.html | changed preference: browser.uiCustomization.state

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: imoraru [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=420317858&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/RHsLQ_7DT8WqRUx5_9NT3A/runs/0/artifacts/public/logs/live_backing.log
Reftest URL: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/RHsLQ_7DT8WqRUx5_9NT3A/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

---

[task 2023-06-22T16:38:22.345Z] 16:38:22     INFO - TEST-START | toolkit/components/extensions/test/mochitest/test_ext_browserAction_getUserSettings.html
[task 2023-06-22T16:38:23.535Z] 16:38:23     INFO - GECKO(1634) | JavaScript error: resource://gre/modules/XULStore.sys.mjs, line 60: Error: Can't find profile directory.
[task 2023-06-22T16:38:23.706Z] 16:38:23     INFO - GECKO(1634) | Console message: [JavaScript Warning: "browser.ui.customized_widgets - The key length must be limited to 72 characters."]
[task 2023-06-22T16:38:23.834Z] 16:38:23     INFO - GECKO(1634) | Console message: [JavaScript Warning: "browser.ui.customized_widgets - The key length must be limited to 72 characters."]
[task 2023-06-22T16:38:23.868Z] 16:38:23     INFO - GECKO(1634) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2023-06-22T16:38:23.869Z] 16:38:23     INFO - GECKO(1634) | MEMORY STAT | vsize 2524MB | residentFast 112MB | heapAllocated 9MB
[task 2023-06-22T16:38:23.876Z] 16:38:23     INFO - TEST-OK | toolkit/components/extensions/test/mochitest/test_ext_browserAction_getUserSettings.html | took 1531ms
[task 2023-06-22T16:38:23.998Z] 16:38:23    ERROR - TEST-UNEXPECTED-FAIL | /tests/toolkit/components/extensions/test/mochitest/test_ext_browserAction_getUserSettings.html | changed preference: browser.uiCustomization.state
[task 2023-06-22T16:38:24.001Z] 16:38:24     INFO - TEST-START | Shutdown
[task 2023-06-22T16:38:24.003Z] 16:38:24     INFO - Passed:  6
[task 2023-06-22T16:38:24.004Z] 16:38:24     INFO - Failed:  0
[task 2023-06-22T16:38:24.006Z] 16:38:24     INFO - Todo:    0
[task 2023-06-22T16:38:24.008Z] 16:38:24     INFO - Mode:    e10s
[task 2023-06-22T16:38:24.009Z] 16:38:24     INFO - Slowest: 1531ms - /tests/toolkit/components/extensions/test/mochitest/test_ext_browserAction_getUserSettings.html
[task 2023-06-22T16:38:24.011Z] 16:38:24     INFO - TEST-INFO | Ran 1 Loops
[task 2023-06-22T16:38:24.012Z] 16:38:24     INFO - SimpleTest FINISHED
[task 2023-06-22T16:38:24.058Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.058Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.058Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.062Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.062Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.062Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.062Z] 16:38:24     INFO - GECKO(1634) | Exiting due to channel error.
[task 2023-06-22T16:38:24.106Z] 16:38:24     INFO - TEST-INFO | Main app process: exit 0
[task 2023-06-22T16:38:24.106Z] 16:38:24     INFO - runtests.py | Application ran for: 0:00:08.537249

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:gregp, since you are the author of the regressor, bug 1814905, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Gregory Pappas [:gregp]]

Investigating.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1814905

Comment 4

[Intermittent Failures Robot]

5 failures in 3880 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 5

Platform and build breakdown:

• linux1804-64-qr: 2
  • debug: 1
  • opt: 1
• macosx1015-64-qr: 2
  • debug: 1
  • opt: 1
• linux1804-64-tsan-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1839948&startday=2023-06-19&endday=2023-06-25&tree=all

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1814905

Comment 6

[Rob Wu [:robwu]]

This is not a regression of bug 1814905. Any extension that declares a browserAction button can trigger this. Here is an example with a different test:

./mach test toolkit/components/extensions/test/mochitest/test_ext_browserAction_onClicked.html --compare-preferences

At the end of the test, the browser.uiCustomization.state pref is changed. When I print the relevant pref values _comparePrefs in SpecialPowersParent.sys.mjs by adding dump(key + " base: " + value+ "\n" + key + " targ " + target.get(key) + "\n\n");
then I see the values (below) with the following changes:

• dirtyAreaCache contains "unified-extensions-area"
• placements["unified-extensions-area"] contains _8b5145fc-aba5-4e68-9a4b-6185691116c2_-browser-action
• seen contains _8b5145fc-aba5-4e68-9a4b-6185691116c2_-browser-action

Clearly, despite uninstallation of an extension, there are still traces of the browserAction button. Upon extension unload (which can happen for any reason: browser shutdown, extension update, extension reload, extension uninstall), the widget destruction is triggered from here: https://searchfox.org/mozilla-central/rev/6b91b922725838e2732aeb478b13e5b33e33ce1b/browser/components/extensions/parent/ext-browserAction.js#169

---

Before extension install:
{"placements":{"widget-overflow-fixed-list":[],"unified-extensions-area":[],"nav-bar":["back-button","forward-button","stop-reload-button","customizableui-special-spring1","urlbar-container","customizableui-special-spring2","save-to-pocket-button","downloads-button","fxa-toolbar-menu-button","unified-extensions-button"],"TabsToolbar":["firefox-view-button","tabbrowser-tabs","new-tab-button","alltabs-button"],"PersonalToolbar":["personal-bookmarks"]},"seen":["save-to-pocket-button","profiler-button","developer-button"],"dirtyAreaCache":["nav-bar"],"currentVersion":19,"newElementCount":2}

After extension install + uninstall:

{"placements":{"widget-overflow-fixed-list":[],"unified-extensions-area":["_8b5145fc-aba5-4e68-9a4b-6185691116c2_-browser-action"],"nav-bar":["back-button","forward-button","stop-reload-button","customizableui-special-spring1","urlbar-container","customizableui-special-spring2","save-to-pocket-button","downloads-button","fxa-toolbar-menu-button","unified-extensions-button"],"TabsToolbar":["firefox-view-button","tabbrowser-tabs","new-tab-button","alltabs-button"],"PersonalToolbar":["personal-bookmarks"]},"seen":["save-to-pocket-button","profiler-button","developer-button","_8b5145fc-aba5-4e68-9a4b-6185691116c2_-browser-action"],"dirtyAreaCache":["nav-bar","unified-extensions-area"],"currentVersion":19,"newElementCount":2}

Comment 7

[Rob Wu [:robwu]]

Gijs, do you know whether the behavior described in comment 6 is intentional?

If it is, would it make sense to add browser.uiCustomization.state to https://searchfox.org/mozilla-central/rev/6b91b922725838e2732aeb478b13e5b33e33ce1b/testing/mochitest/ignorePrefs.json ? I did something similar for pageactions in bug 1839890.

Comment 8

[:Gijs (he/him)]

(In reply to Rob Wu [:robwu] from comment #7)

Gijs, do you know whether the behavior described in comment 6 is intentional?

It was when the CUI code was originally written. Specifically, it meant that in the situation:

1. install extension. Extension adds itself to the toolbar.
2. user removes extension icon
3. extension updates / is uninstalled and reinstalled / whatever

the CUI code could avoid adding the extension to the toolbar again. This happens here.

I think we should probably re-evaluate this system in light of the new unified extensions button, where nothing is "pinned" to anywhere (other than the unified extensions menu where it has to show up) by default anyway.

If it is, would it make sense to add browser.uiCustomization.state to https://searchfox.org/mozilla-central/rev/6b91b922725838e2732aeb478b13e5b33e33ce1b/testing/mochitest/ignorePrefs.json ? I did something similar for pageactions in bug 1839890.

I think for now yes. It would be useful to have a follow-up for the implications here. CUI is really over-complicated for the 2023 state of the world, and we should tear some of that complexity out at this point, but it'd be good to understand the requirements from the webextension side of things before touching it (and it's probably a bigger project anyway). Would you mind filing a bug to this effect that outlines what our current expectations are for webextension buttons and what support this needs from CUI?

Also worth noting that all these areas getting marked as dirty is non-ideal but a known issue, cf. bug 1278176 and bug 1395983.

Comment 9

[Rob Wu [:robwu]]

(In reply to :Gijs (he/him) from comment #8)

(In reply to Rob Wu [:robwu] from comment #7)

Gijs, do you know whether the behavior described in comment 6 is intentional?

It was when the CUI code was originally written. Specifically, it meant that in the situation:

1. install extension. Extension adds itself to the toolbar.
2. user removes extension icon
3. extension updates / is uninstalled and reinstalled / whatever

the CUI code could avoid adding the extension to the toolbar again. This happens here.

I think we should probably re-evaluate this system in light of the new unified extensions button, where nothing is "pinned" to anywhere (other than the unified extensions menu where it has to show up) by default anyway.

A button can still be pinned if the extension opts in to it, or through enterprise policies. Coincidentally the topic came up again today (https://github.com/mozilla/web-ext/issues/2874#issuecomment-1739216681), where we expressed support for adding a pref to allow users to configure the install-time default.

If it is, would it make sense to add browser.uiCustomization.state to https://searchfox.org/mozilla-central/rev/6b91b922725838e2732aeb478b13e5b33e33ce1b/testing/mochitest/ignorePrefs.json ? I did something similar for pageactions in bug 1839890.

I think for now yes. It would be useful to have a follow-up for the implications here. CUI is really over-complicated for the 2023 state of the world, and we should tear some of that complexity out at this point,

I saw a "Remove browser.uiCustomization.state" bug before, but it just got closed - bug 1209757. I don't have enough context to file a follow-up bug for that.

but it'd be good to understand the requirements from the webextension side of things before touching it (and it's probably a bigger project anyway). Would you mind filing a bug to this effect that outlines what our current expectations are for webextension buttons and what support this needs from CUI?

Given my reply above (about there being situations where the button is still pinned at install), does that change anything?

Comment 10

[Rob Wu [:robwu]]

Attachment: Bug 1839948 - Add browser.uiCustomization.state to ignorePrefs.json

See https://bugzilla.mozilla.org/show_bug.cgi?id=1839948#c6

Comment 11

[Intermittent Failures Robot]

2 failures in 3726 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 2

Platform and build breakdown:

• linux1804-64-qr: 2
  • debug: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1839948&startday=2023-09-25&endday=2023-10-01&tree=all

Comment 12

[:Gijs (he/him)]

(In reply to Rob Wu [:robwu] from comment #9)

(In reply to :Gijs (he/him) from comment #8)

It would be useful to have a follow-up for the implications here. CUI is really over-complicated for the 2023 state of the world, and we should tear some of that complexity out at this point,

I saw a "Remove browser.uiCustomization.state" bug before, but it just got closed - bug 1209757. I don't have enough context to file a follow-up bug for that.

Well, the premise for 1209757 was that we could just base all customization state on the contents of XULStore, and I don't think that's true at the moment or at the time the bug was filed. Of course, given enough engineering effort we could move all the state from the pref into xulstore, but that doesn't really affect the contents of the state per se, so it's orthogonal to the discussion here other than that we happen to have enforcement mechanisms for tests changing pref state and not for tests changing xulstore state. If we accept the premise that the test changing state is bad (because it might break later tests in the same manifest run), in what file it changes the state is kind of immaterial. :-)

but it'd be good to understand the requirements from the webextension side of things before touching it (and it's probably a bigger project anyway). Would you mind filing a bug to this effect that outlines what our current expectations are for webextension buttons and what support this needs from CUI?

Given my reply above (about there being situations where the button is still pinned at install), does that change anything?

Yes, so, I think what we're running into here is that I don't really understand what the requirements are from the webextension side at this point, and you're not sure what if anything could change in CUI. :-)

We could have a meeting, or do an async google doc to align. Perhaps the doc is better because in theory if we do a reasonable job we could export it into fx-source-docs and it'd be useful in future for people wanting to understand how customization and extensions work together? WDYT?

Comment 13

[Rob Wu [:robwu]]

(In reply to :Gijs (he/him) from comment #12)

Yes, so, I think what we're running into here is that I don't really understand what the requirements are from the webextension side at this point, and you're not sure what if anything could change in CUI. :-)

We could have a meeting, or do an async google doc to align. Perhaps the doc is better because in theory if we do a reasonable job we could export it into fx-source-docs and it'd be useful in future for people wanting to understand how customization and extensions work together? WDYT?

Sounds good; I filed bug 1856351 as a follow-up so that it doesn't get lost when this bug is closed.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56e2ffbd4ed3
Add browser.uiCustomization.state to ignorePrefs.json r=Gijs

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/56e2ffbd4ed3