[FlyWeb] Fix orange in Mochitest BC5 (browser_BrowserUITelemetry_defaults.js)

RESOLVED FIXED

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: djvj, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The test in "browser/modules/test/browser_BrowserUITelemetry_defaults.js" is failing.  The line it's failing on is:

  Assert.ok(CustomizableUI.inDefaultState,
            "No other test should have left CUI in a dirty state.");

This seems like it's most likely caused by the code FlyWeb adds to CustomizableWidgets.jsm, adding a new FlyWeb toolbar button.  But I'm not sure how adding that button causes this "inDefaultState" invocation to fail.
(Reporter)

Comment 1

2 years ago
Hi Mark,

I did a blame on the test file ("browser_BrowserUITelemetry_defaults.js") and found a commit by you, so I'm figuring you're the best person to ask.

Could you give some insight on what might be causing this bug and how I could fix it?  For reference, here is the code we added to CustomizableWidgets.jsm:

https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1272107&attachment=8751443

Could you give me a hint as to how adding this widget could cause "CustomizableUI.inDefaultState" to return false?
Flags: needinfo?(markh)
Ah - this might be because the FlyWeb button is adding itself to the menu panel by default via the defaultArea property in its bootstrap.js. Because there's no corresponding updates to the list of default widgets in CustomizableUI, I suspect BrowserUITelemetry is interpreting (correctly) that an add-on is adding a new button, which this doesn't expect.

We might try removing the defaultArea property from the button definition. That'll leave FlyWeb in the customization palette, which the user can then drag into one of their toolbars / panel if / when they're ready to use it.
(Reporter)

Comment 3

2 years ago
Thanks mike!  I've removed the defaultArea and pushed to try.
(Reporter)

Comment 5

2 years ago
Created attachment 8756997 [details] [diff] [review]
fix-customizableui-defaultstate.patch
Attachment #8756997 - Flags: review?(jdarcangelo)
(Reporter)

Updated

2 years ago
Flags: needinfo?(markh)
Attachment #8756997 - Flags: review?(jdarcangelo) → review+
Note that IIUC, your patch will mean that the button is not in the toolbar by default. If you really want it to be, you probably need that test to treat your new button in the same way that the social-share-button is, then either open another bug like 1273358, or change to scope of that bug to handle all such buttons. See also bug 1268349 comment 6.
(Reporter)

Comment 7

2 years ago
Thanks mark.  I think I'll take it out of the toolbar by default for now, to get our treeherder green, which will let us land.  We can always add it in post-landing, ensuring that we don't push things red.  Right now the landing is higher priority than making sure it's default, since it's a harder deadline to meet.
(Reporter)

Comment 8

2 years ago
http://hg.mozilla.org/projects/larch/rev/545da344ffbd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.