1583229 - Remove unecessary .bookmarks-toolbar rule from widget-overflow and appmenu-popup elements.

Status: REOPENED

(Firefox :: Toolbars and Customization, task, P3)

Description

[:Gijs (he/him)]

After bug 1562881 the lists at these two selections: https://searchfox.org/mozilla-central/rev/e3fc8f8970491aef14d3212b2d052942f4d29818/browser/themes/shared/customizableui/panelUI.inc.css#141-149,166-172 are out of sync.

We should probably also ensure that the protection popup actually gets the requisite class when the bookmarks toolbar is present.

Comment 1

[Johann Hofmann [:johannh]]

Erica has worked on popup height before, can you please take a look at this? Thanks!

Comment 2

[Erica Wright [:ewright]]

The rule at line 166-172 is meant for when the icon/anchor is located in the bookmarks toolbar. For example, the library icon in the image I've attached. As far as I can tell, I'm unable to move the urlbar into the bookmarks toolbar, and since the shield is located inside the urlbar, this rule should never need to apply to the shield or protections popup.

I'll close this as invalid for now, please let me know if I've misunderstood your report.

Comment 3

[Erica Wright [:ewright]]

Attachment: Screen Shot 2019-10-01 at 11.17.03 AM.png

Comment 4

[:Gijs (he/him)]

(In reply to Erica Wright [:ewright] from comment #2)

The rule at line 166-172 is meant for when the icon/anchor is located in the bookmarks toolbar. For example, the library icon in the image I've attached. As far as I can tell, I'm unable to move the urlbar into the bookmarks toolbar, and since the shield is located inside the urlbar, this rule should never need to apply to the shield or protections popup.

I'll close this as invalid for now, please let me know if I've misunderstood your report.

I think that's fine, but then we should probably remove the widget-overflow and appmenu-popup ones from the .bookmarks-toolbar set of selectors given that they can never be in that toolbar either, and potentially document what that's doing more clearly in the comment already above it?

Comment 5

[Erica Wright [:ewright]]

I agree, we can rename this bug for that.

Comment 6

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

Is this meant for 71? Just double checking.

Comment 7

[Erica Wright [:ewright]]

(In reply to Liz Henry (:lizzard) from comment #6)

Is this meant for 71? Just double checking.

This is low priority, it doesn't need to be in for 71.

Comment 8

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

The priority flag is not set for this bug.
:johannh, could you have a look please?

For more information, please visit auto_nag documentation.