browser_BrowserUITelemetry_defaults.js fails when screenshots is enabled in tests

NEW
Unassigned

Status

()

Firefox
Toolbars and Customization
8 months ago
8 months ago

People

(Reporter: standard8, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
We're looking at enabling Screenshots in mochitests by default. In browser_BrowserUITelemetry_defaults.js we're seeing this test failure:

1879 INFO TEST-UNEXPECTED-FAIL | browser/modules/test/browser/browser_BrowserUITelemetry_defaults.js | No other test should have left CUI in a dirty state. - false == true - JS frame :: chrome://mochitests/content/browser/browser/modules/test/browser/browser_BrowserUITelemetry_defaults.js :: test :: line 15
Stack trace:
chrome://mochitests/content/browser/browser/modules/test/browser/browser_BrowserUITelemetry_defaults.js:test:15
chrome://mochikit/content/browser-test.js:Tester_execTest:771
chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:672
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:791
Buffered messages finished
(Reporter)

Comment 1

8 months ago
I have a patch for this bug, however I think it depends on bug 1356256 as we're currently seeing inconsistent start-ups for Screenshots in tests.
(Reporter)

Comment 2

8 months ago
(In reply to Mark Banner (:standard8) from comment #1)
> I have a patch for this bug, however I think it depends on bug 1356256 as
> we're currently seeing inconsistent start-ups for Screenshots in tests.

Ok, I was wrong about that, I think we should land this now to avoid issues with restore defaults (Screenshots is currently disabled by default in tests, we'll get it enabled soon).
Comment hidden (mozreview-request)

Comment 4

8 months ago
mozreview-review
Comment on attachment 8860145 [details]
Bug 1358097 - Add Screenshots to the navbar placements for Customizable UI when Screenshots is enabled.

https://reviewboard.mozilla.org/r/132176/#review135272

::: browser/components/customizableui/CustomizableUI.jsm:260
(Diff revision 1)
> +    // Firefox Screenshots is a WebExtension, and WebExtensions place buttons
> +    // at the end of the toolbar. Apparently this happens after pocket's
> +    // createWidget call (see previous comment).

This sounds like a race condition waiting to happen, especially in the "migration" case for pocket (for profiles that had the add-on before).

We should stop hacking around this and fix bug 1247045 instead. Now that we're deprecating non-webextension buttons we can just change the API to no longer prevent opting in to "real" default buttons.
Attachment #8860145 - Flags: review?(gijskruitbosch+bugs)
(Reporter)

Updated

8 months ago
Assignee: standard8 → nobody
Depends on: 1247045
You need to log in before you can comment on or make changes to this bug.