1111986 - Bookmark toolbar being pushed into the expander when put next to address bar

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[pablo.bollansee]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141125180439

Steps to reproduce:

To save vertical space I put the bookmarks toolbar next to my address bar, like this: http://i.imgur.com/OWxykWL.png I then try to drag one of the bookmarks somewhere else, or drag from the address bar to the bookmarks bar to create a new bookmark.

The easiest way I found to get the bug is to click and drag one of the bookmarks off the toolbar (but keep dragging) and then back onto it. Sometimes I have to do this a few times before getting the bug.


Actual results:

The bookmarks toolbar gets pushed into the expander like this: http://i.imgur.com/KVLgcdn.png ; http://i.imgur.com/qmUVCq1.png


Expected results:

The bookmark should be moveable without the bookmarks toolbar disappearing.

Comment 1

[u279076]

Pablo, I'm not able to reproduce this behaviour. Can you please try running Firefox with a new profile to see if it still happens?

https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

Comment 2

[pablo.bollansee]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #1)
> Pablo, I'm not able to reproduce this behaviour. Can you please try running
> Firefox with a new profile to see if it still happens?

I made a new profile and moved the bookmarks bar next to the address bar like in my own setup. Then I tried to reproduce the problem, but indeed it did not happen. However, I then added a few bookmarks so there were too much to fit on the bar, and the >> appeared. I then tried it again and the bookmarks bar got pushed away.

It seems that it works fine as long as the bookmarks bar can show all the bookmarks, once there are too many it breaks.

Comment 3

[u279076]

I tried what you described in comment 2 and I'm still not seeing this. Can you provide a screencast video showing how you reproduce this from a clean browser state?

Comment 4

[pablo.bollansee]

I made a video as you asked. I uploaded it to youtube: https://www.youtube.com/watch?v=6p8cIZ3QBHM&feature=youtu.be

Comment 5

[u279076]

Thanks for providing the video, it made the steps to reproduce much clearer for me. Unfortunately I'm still unable to reproduce this. The only difference I can see is that I'm using Mac OS 10.10 and you appear to be on a previous version. What version of OS X do you have installed?

Comment 6

[pablo.bollansee]

I'm on OS X 10.8.5. It does sometimes takes more moving-the-bookmark-around than other times, though I always get it within seconds.

Thanks for all the help already!

Comment 7

[:Gijs (he/him)]

Attachment: fix bookmarks toolbar items overflow triggering navbar overflow,

When I debugged this (thanks for the video, that really helped), I noticed that the events we get from the bookmarks toolbar item always had a rangeParent (which pointed to the bookmarks toolbar item container). I considered checking rangeParent against the bookmarks toolbar items container, but then I realized that that would be pretty fragile if we ever change that implementation, and wouldn't work for add-ons. So I did this. It works. I don't fully understand how, but I think it's a workable solution. Thoughts?

Comment 8

[Marco Mucci [:MarcoM]]

Hi Gijs, can you provide a point value.

Comment 9

[:Gijs (he/him)]

(In reply to pablo.bollansee from comment #6)
> I'm on OS X 10.8.5. It does sometimes takes more moving-the-bookmark-around
> than other times, though I always get it within seconds.
> 
> Thanks for all the help already!

Pablo, thank you for your help in getting details about this issue!

I created a try build:

https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-57452ce04551

there should be a build there in about 3 hours (maybe a bit less, maybe a bit more?). It would be helpful if you can see if that works for you or if it only fixes the really obvious dragging case, but still breaks when you don't expect it to.

(note that this will be a nightly build (based on one, anyway), so to avoid breaking things in your main profile, you may want to create a new one and use Firefox sync to quickly get the same bookmarks and so on - all kinds of other things might be a bit different from the version of Firefox you normally use)

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8548418 [details] [diff] [review]
fix bookmarks toolbar items overflow triggering navbar overflow,

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

I reproduced this and looked at it in the debugger. This change looks OK to me.

::: browser/components/customizableui/CustomizableUI.jsm
@@ +3970,5 @@
>  
>    onOverflow: function(aEvent) {
>      if (!this._enabled ||
> +        (aEvent && aEvent.target != this._toolbar.customizationTarget) ||
> +        (aEvent && aEvent.rangeParent))

I thought about sharing the aEvent null-check between the two conditions but the code gets pretty ugly when doing so.

This is OK but please include a comment saying that the check for 'rangeParent' is used to determine if this is an item that is moving within a container like the Bookmark Toolbar Items.

Comment 11

[pablo.bollansee]

@Gijs I tried the build you made and it indeed fixed the bug.

I did get it once more when the new 'Firefox Hello' and 'Share This Page' button appeared next to the bar while I was dragging. The buttons appeared because they were new (I was still on FF34). I'm not sure how to reproduce this, because it only happened when the buttons appeared by themselves.

Other than that, nothing I did would make the bookmarks bar disappear.

One more thing, how long will it take for this fix to find its way into the official release? Or is it safe to continue using the nightly build?

Thanks a lot for all the help!

Comment 12

[:Gijs (he/him)]

w/ comment:

remote:   https://hg.mozilla.org/integration/fx-team/rev/56c85ebbf6d0

(In reply to pablo.bollansee from comment #11)
> @Gijs I tried the build you made and it indeed fixed the bug.
> 
> I did get it once more when the new 'Firefox Hello' and 'Share This Page'
> button appeared next to the bar while I was dragging. The buttons appeared
> because they were new (I was still on FF34). I'm not sure how to reproduce
> this, because it only happened when the buttons appeared by themselves.
> 
> Other than that, nothing I did would make the bookmarks bar disappear.

Thanks! This is very useful.

> One more thing, how long will it take for this fix to find its way into the
> official release? Or is it safe to continue using the nightly build?

Assuming the change sticks, it will take quite a while - this is kind of the "worst" moment to land changes - namely a full 17-18 weeks, until this hits a release build.

It is 'safe' in the sense that it is alpha software. It may be a lot crashier, we test things like e10s (process separation between web content and the browser) and new video implementation stuff there, and so if you expect the software to be a lot more stable than that, it might not be for you. Alternatively, you could consider using developer edition ( https://aurora.mozilla.org) or beta ( https://beta.mozilla.org ) which will see this fix in 6 or 12 weeks, assuming all is well.


The other option that is potentially open here is that we fast-track the change. It's a pretty small change, so from that perspective it is quite possible, but on the other hand, it affects a relatively small minority of users. Uplifting it later this week to developer edition seems feasible, though. Jared, does that sound right?

Comment 13

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #12)
> w/ comment:
> 
> remote:   https://hg.mozilla.org/integration/fx-team/rev/56c85ebbf6d0
> 
> (In reply to pablo.bollansee from comment #11)
> > @Gijs I tried the build you made and it indeed fixed the bug.
> > 
> > I did get it once more when the new 'Firefox Hello' and 'Share This Page'
> > button appeared next to the bar while I was dragging. The buttons appeared
> > because they were new (I was still on FF34). I'm not sure how to reproduce
> > this, because it only happened when the buttons appeared by themselves.
> > 
> > Other than that, nothing I did would make the bookmarks bar disappear.
> 
> Thanks! This is very useful.

So, Jared, one more thing... this (the buttons appearing making the *bookmarks* (not just the buttons) disappear) makes me worry (something which I've wondered before) if there are cases where the overflow detection code is broken. :-(

By which I mean, the code that checks the widths of items vs. the available space (or rather, asks layout about the amount of space by which the overflow-x'd container can "scroll") sometimes returns non-0 when really, the answer should be 0...

If that is the case though, I don't really have a good idea of how to debug this and/or figure out what is causing this, as it seems to be racy and therefore doesn't lend itself well to debugging / inspecting things while the code is running.

See bug 1021303 comment 6 for more evidence about this. I don't really understand how layout can be "not settled yet" - but it might be worth pursuing that issue in more depths on that bug.

Comment 14

[pablo.bollansee]

@Gijs

No problem, happy to help :D
Thanks for the info about releases, I think I'll stay with the official release then. I don't drag booksmarks around too much so that's ok.

> ... if there are cases where the overflow detection code is broken. ...
This indeed sounds like something that could cause it. I however can't help debugging either :(

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/56c85ebbf6d0

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

The change seems pretty low risk for uplift, but also not something that is encountered often so I'm indifferent.

As to the quirks with overflow, without a reduced testcase and steps to reproduce, I'm not sure what else we can do there. At the moment it seems like a platform issue that we will have to live with until we can reproduce it more reliably.

Comment 17

[u279076]

As per comment 5 this is not reproducible in QA so it's not something we'll be able to verify is fixed.

Comment 18

[pablo.bollansee]

I encountered this bug again in the latest Firefox (57.0 (64-bit)) on Windows 10. Same steps to reproduce as before.