Bookmark toolbar items overflow completely instead of only the not fitting ones, if on the Navigation Bar (since Fx 31)

VERIFIED FIXED in Firefox 33

Status

()

Firefox
Toolbars and Customization
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: exa.atto1818, Assigned: Sang Mercado, Mentored)

Tracking

({regression})

31 Branch
Firefox 34
x86_64
All
regression
Points:
---
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox33 verified, firefox34 verified)

Details

(Whiteboard: [good first bug][lang=css])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 8461374 [details]
FX 31

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140716183446

Steps to reproduce:

1. Place Bookmark Toolbar Items in nav-bar, between url-bar and search-bar.
2. Put so many bookmarks into the Bookmark Toolbar Items that they cannot be displayed in nav-bar.


Actual results:

The Bookmark Toolbar Items and search-bar got "over-flow" outside, only leaving a very long url-bar and a "More Tools..." button in nav-bar.


Expected results:

As in FX 30 and before, the extra bookmarks are overflowed out while the search-bar remained in the nav-bar.
(Reporter)

Comment 1

4 years ago
Created attachment 8461375 [details]
FX 30 and before
(Reporter)

Comment 2

4 years ago
I believe this guy have the same problem with me:
https://support.mozilla.org/ta/questions/1012109
(Reporter)

Updated

4 years ago
Component: Untriaged → Toolbars and Customization

Updated

4 years ago
Keywords: regressionwindow-wanted

Comment 3

4 years ago
Regression window:

-g 2014-04-26-03-02-04-mozilla-central-firefox-31.0a1.en-US.linux-x86_64 0e91262606a6
-b 2014-04-27-03-02-04-mozilla-central-firefox-31.0a1.en-US.linux-x86_64 c67a79064fd4

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e91262606a6&tochange=c67a79064fd4

49ec0d4753dd	Jared Wein — Bug 940174 - "Bookmarks Toolbar Items" centers its contents when placed in the tabstrip. r=mikedeboer
Blocks: 940174
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regressionwindow-wanted → regression
OS: Windows 8.1 → All
Summary: nav-bar bookmark-toolbar-items overflow manner in FX 31 → nav-bar bookmark-toolbar-items overflow manner in Fx 31

Updated

4 years ago
Duplicate of this bug: 1045040

Updated

4 years ago
Duplicate of this bug: 1046764

Updated

4 years ago
Flags: needinfo?(jaws)
Summary: nav-bar bookmark-toolbar-items overflow manner in Fx 31 → Placed on navigation bar, Bookmark toolbar items overflow completely instead of only the not fitting ones (since Fx 31)

Updated

4 years ago
Flags: needinfo?(mdeboer)
Summary: Placed on navigation bar, Bookmark toolbar items overflow completely instead of only the not fitting ones (since Fx 31) → Bookmark toolbar items overflow completely instead of only the not fitting ones, if on the Navigation Bar (since Fx 31)
I see the problem here. The patch in bug 940174 moved the flex=1 off of the element and added some CSS that only applies flex when it is the direct child of a toolbar that is not the TabsToolbar. Except when it is in the nav-bar, it is actually a direct child of the #nav-bar-customization-target, not #nav-bar, and so the flex isn't applied when it is in the nav-bar.

Updating that rule to add #nav-bar-customization-target { -moz-box-flex: 1; } would fix it.

Gavin, should this be added to the next iteration? It should be an easy fix, although it is a very low severity and should also be a low frequency bug.
Flags: needinfo?(mdeboer) → needinfo?(gavin.sharp)
Sounds like a good mentored/first bug? We can certainly add to the backlog though I don't see it hitting the prioritized backlog soon for the reasons you mention. Doesn't mean we can't fix it!
Flags: needinfo?(jaws)
Flags: needinfo?(gavin.sharp)
Flags: firefox-backlog+
Yeah, I can mentor it. Please see comment #6 for more information about what is necessary to fix it. The place to make this change is at http://hg.mozilla.org/mozilla-central/annotate/7fc96293ada8/browser/base/content/browser.css#l294
Mentor: jaws
Flags: needinfo?(jaws)
Whiteboard: [good first bug][lang=css]
(Assignee)

Comment 9

4 years ago
Hi, I would like to be assigned to this bug.

Updated

4 years ago
Assignee: nobody → sang.mercado
(Assignee)

Comment 10

4 years ago
Created attachment 8474200 [details] [diff] [review]
bug1043257.dff

First patch. Not sure if I did everything correct
Attachment #8474200 - Flags: review?(jaws)
Comment on attachment 8474200 [details] [diff] [review]
bug1043257.dff

Review of attachment 8474200 [details] [diff] [review]:
-----------------------------------------------------------------

Were you able to test your changes? Please make sure that your changes work before requesting review. If you need help with getting your changes built so you can test them, you can join #introduction on irc.mozilla.org and someone there can help you with getting your build up-and-running.

::: browser/base/content/browser.css
@@ +289,5 @@
>  #personal-bookmarks[cui-areatype="toolbar"][overflowedItem=true] > #bookmarks-toolbar-placeholder {
>    display: -moz-box;
>  }
>  
> +#nav-bar-customization-target,

This should be:
#nav-bar-customization-target > #personal-bookmarks,
Attachment #8474200 - Flags: review?(jaws) → review-
(Assignee)

Comment 12

4 years ago
Created attachment 8475365 [details] [diff] [review]
Bug1043257.diff
Attachment #8475365 - Flags: review?(jaws)
(Assignee)

Comment 13

4 years ago
Created attachment 8475368 [details] [diff] [review]
Bug1043257.diff
Attachment #8474200 - Attachment is obsolete: true
Attachment #8475365 - Attachment is obsolete: true
Attachment #8475365 - Flags: review?(jaws)
Attachment #8475368 - Flags: review?(jaws)
(Assignee)

Updated

4 years ago
Attachment #8475368 - Flags: checkin+
Comment on attachment 8475368 [details] [diff] [review]
Bug1043257.diff

This may need a try run before landing. I'll push it to tryserver for you so we can verify that it doesn't break any tests.

After the try results confirm that everything is good, then we can set the checkin-needed keyword on this bug.
Attachment #8475368 - Flags: checkin+
(Assignee)

Comment 15

4 years ago
Is there anything else to do on my part?
Try push: https://tbpl.mozilla.org/?tree=Try&rev=4cb594586a22

(In reply to Sang Mercado from comment #15)
> Is there anything else to do on my part?

At this point, no. Just sit back and wait for the builds to finish and to see that all the tests pass :)
Congrats on your first patch. I just checked it in, and it should show up in Firefox Nightly tomorrow or the day after.

https://hg.mozilla.org/integration/fx-team/rev/9dd74fe0d194
Keywords: checkin-needed
Whiteboard: [good first bug][lang=css] → [good first bug][lang=css][fixed in fx-team]
https://hg.mozilla.org/mozilla-central/rev/9dd74fe0d194
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=css][fixed in fx-team] → [good first bug][lang=css]
Target Milestone: --- → Firefox 34

Comment 19

4 years ago
Apologies if this information is somewhere on this page, because I can't find it: is this patch going to be uplifted to Firefox 32 (beta) and Firefox 33 (Aurora)? We still have questions about it coming in on SuMo.
Comment on attachment 8475368 [details] [diff] [review]
Bug1043257.diff

It's too late to get it in the for Firefox 32 beta, but I am requesting uplift for Aurora 33.

Approval Request Comment
[Feature/regressing bug #]: regression caused by bug 940174
[User impact if declined]: users who have the bookmarks toolbar item in the navbar may not see any bookmarks if they have a lot of bookmarks in the item
[Describe test coverage new/current, TBPL]: no automated test coverage
[Risks and why]: no risks expected
[String/UUID change made/needed]: none
Attachment #8475368 - Flags: approval-mozilla-aurora?
status-firefox33: --- → affected
status-firefox34: --- → fixed
Attachment #8475368 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [good first verify]
Flags: qe-verify+

Updated

4 years ago
Iteration: --- → 34.3
Reproduced the issue on Nightly 2014-07-11. 

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.4 using latest Nightly 34.0a1 (buildID: 20140828030205) and latest Aurora 33.0a2 (buildID: 20140829004003).
Status: RESOLVED → VERIFIED
status-firefox33: fixed → verified
status-firefox34: fixed → verified
QA Contact: camelia.badau
You need to log in before you can comment on or make changes to this bug.