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

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Kannan Vijayan [:djvj]]

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.

Comment 1

[Kannan Vijayan [:djvj]]

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?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 3

[Kannan Vijayan [:djvj]]

Thanks mike!  I've removed the defaultArea and pushed to try.

Comment 4

[Kannan Vijayan [:djvj]]

Seems to work: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e751bdd2f69

Comment 5

[Kannan Vijayan [:djvj]]

Attachment: fix-customizableui-defaultstate.patch

Comment 6

[Mark Hammond [:markh] [:mhammond]]

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.

Comment 7

[Kannan Vijayan [:djvj]]

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.

Comment 8

[Kannan Vijayan [:djvj]]

http://hg.mozilla.org/projects/larch/rev/545da344ffbd