1435705 - Options UI hidden

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[marius.santa]

Attachment: options not showing up.gif

[Affected versions]:
- Firefox 58.0.1 (20180128191252)
- Firefox 59.0b6 (20180201171410)
- Firefox 60.0a1 (20180205100240)

[Affected platforms]:
- Win 7 64-bit
- Mac OS X 10.13.1

[Steps to reproduce]:
1.Install an extension that has an Options UI panel, for example: https://addons.mozilla.org/en-US/firefox/addon/colorfultabs/
2.Go to the about:addons>Extensions.
3.Click on the "More" button for the previously installed extension.

[Expected results]:
The extension's details page is displayed along with the Options UI panel.

[Actual results]:
The Options UI panel is hidden.

Notes:
This happens with other extensions as well.
It seems that after the extensions details page is opened a "refresh" is performed that triggers the hiding of the Options UI panel.
Gif is attached.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

I took a look at this and the reason for the issue seems to be related to the discovery panel view,
a STR to reproduce it consistently is:

- install an extension with an options_ui page (e.g. the one linked in Comment 0)
- open the about:addons page in a tab
- open the developer tools on the about:addons page and enable the option "Disable HTTP Cache (when toolbox is open)"
- reload the about:addons tab (to be sure that the discovery panel has not been loaded yet)
- switch to the addon details of the installed extension and verify that the options_ui page is rendered as expected
- switch to the discovery panel and then switch back to the addon details view before the discovery panel has been
  loaded
- verify that the options_ui page is initially rendered, and then it is removed (as soon as the discovery panel
  loading has been completed in the background).

The options_ui page is being removed because when the discovery view loading has been completed a ViewChanged
event is sent, and the same event is also received when the user switches from the addon detail to another
about:addons view (e.g. the list of all the installed addons), and in that case we want to remove the 
options_ui rendered for the previosly selected addon:

- https://searchfox.org/mozilla-central/rev/d03ad8843e3bf2e856126bc53b0475c595e5183b/toolkit/mozapps/extensions/content/extensions.js#2176-2185
- https://searchfox.org/mozilla-central/rev/d03ad8843e3bf2e856126bc53b0475c595e5183b/toolkit/mozapps/extensions/content/extensions.js#3205-3207

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1435705 - Fix WebExtensions options_ui hidden.

If a user opens the about:debugging page and switches
to the addon details while the discovery panel is still
loading, a ViewChanged event is received once the discovery
panel has been completely loaded and the options_ui of the
extension was removed (because the ViewChanged event is also
sent when the about:addons page navigates between its sub-views).

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

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

I'm marking this issue as "P2 - minor" (mostly because the issue happens only when the discovery panel has not been cached yet).

I've also pushed the proposed fix attached to this issue on try (to check if it is breaking on any of the existent tests):

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ea64e9d7d1e

None of the oranges seems to be related to this change (and they passed by re-executing them once).

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

Hi Andrew, do you mind to take a look at the proposed fix attached here?

Unfortunately the issue is triggered by a race condition between the first load of the discovery panel and the options ui page rendering while loading the addon detail view, and so I've not added a new test (at least not yet).

Comment 5

[Andrew Swan [:aswan]]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

https://reviewboard.mozilla.org/r/220128/#review226952

Looks good, thanks.  I agree that its not really feasible to add a new test for this.

::: commit-message-3ee38:3
(Diff revision 1)
> +Bug 1435705 - Fix WebExtensions options_ui hidden.
> +
> +If a user opens the about:debugging page and switches

This should say about:addons right?

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

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

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49f6727e3e08
Fix WebExtensions options_ui hidden. r=aswan

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/49f6727e3e08

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this something you think would be safe to uplift for 59? 
Is it widespread (i.e. true for any add-on with options?)

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #10)
> Is this something you think would be safe to uplift for 59? 
> Is it widespread (i.e. true for any add-on with options?)

Yes, the issue is true for any addon with options (but I'm not sure how easy is to trigger it for a real user,
because it is triggered only if the user switch to the addon details view while the discovery panel is still
loading).

My main concern is that we don't have a test for it, and so I think it would be also reasonable to have it verified by QA on Nightly asap, and then on Beta if uplifted.

The fix itself is pretty small and it should be pretty low risky, and so uplifting it sounds good to me
(and I also double-checked with Andrew about it and uplifting this fix to 59 sounds good to him too).

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Sounds like an edge case, so I'll mark this as fix-optional for 59. 
Please request uplift though - we could still get this into Thursday's beta 14 build.

Comment 13

[CosminB]

Attachment: Bug1435705.png

I can reproduce this issue with the steps from comment 1 on Firefox 58.0.2 (20180206200532) under Win 7 64-bit.

This issue is verified as fixed on Firefox 60.0a1 (20180227220155) under Windows 7 64-bit and Mac OS X 10.13.2.

Please see the attached screenshot.

Comment 14

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

Approval Request Comment

[Feature/Bug causing the regression]:

It doesn't seem to be a recent regression, based on some digging into the hg history it seems that the "ViewChanged" event listener which removes the browser XUL element which contains the addon options ui has been there since the initial version of the WebExtensions options UI (introduced in Firefox 48 by Bug 1250784, https://hg.mozilla.org/mozilla-central/diff/c058cdf47547/toolkit/mozapps/extensions/content/extensions.js#l1.164) 

[User impact if declined]:

In the Firefox Desktop about:addons page, the addon options ui embedded in the addon details view are removed if the user switches to the addon details view while the discovery panel is still loading.
It is worth to mention that we are not sure how easy is for a user to actually trigger the issue, and so this looks like an optional "nice to have fix" to cover an edge case.

[Is this code covered by automated tests?]:

The about:addons page is covered by a number of automated tests, but none of them covers this particular scenario.

[Has the fix been verified in Nightly?]:

Yes

[Needs manual test from QE? If yes, steps to reproduce]: 

Yes, the step to reproduce are described in Comment 1.

[List of other uplifts needed for the feature/fix]:

None

[Is the change risky?]:

Low risks

[Why is the change risky/not risky?]:

The risks seems to be pretty low: the change is small and restricted to a very small part of the about:addons page sources.

[String changes made/needed]:

None

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8950878 [details]
Bug 1435705 - Fix WebExtensions options_ui hidden.

Fix for a UI regression, looks ok for uplift for beta 14.

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d4192f91c60

Comment 17

[marius.santa]

Attachment: options ui.PNG

This issue is verified as fixed on Firefox 59.0b14 (20180301022608) under Windows 10 64-bit and Mac OS X 10.13.2.