dragging site icon to bookmark toolbar resizes bookmark toolbar

RESOLVED FIXED in Firefox 3.7a1



9 years ago
9 years ago


(Reporter: asa, Assigned: mak)



Firefox 3.7a1
Mac OS X
Dependency tree / graph
Bug Flags:
blocking-firefox3.6 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)


(Whiteboard: [fixed by bug 512353])


(1 attachment)



9 years ago
Created attachment 399150 [details]
ogg video showing resizing bookmark toolbar height

If you drag the site icon from the addressbar to the bookmarks toolbar, the toolbar height grows 1 pixel.  This seems to be the result of the drop marker height. (Dragging to a folder, where you get no drop marker) on the bookmarks toolbar does not increase the height)

This has been around for a while. I've tested with old and new profiles and with small and large toolbar icons. 

The attached video demonstrates the problem.
Flags: blocking-firefox3.6?
I think this has been caused by bug 382466.
Blocks: 382466
Keywords: regression


9 years ago
Attachment #399150 - Attachment description: bookmark toolbar drop marker too tall → ogg video showing resizing bookmark toolbar height

Comment 2

9 years ago
the toolbar just needs a min-height: 24px;

btw i have a patch for this in bug 512353
I don't think we want to make the toolbar higher.

Comment 4

9 years ago
why higher? the drop indicator is 17px, the toolbar has 5px of padding and 2px of border.
I don't understand. The problem in this bug is that the toolbar becomes higher if the drop indicator is visible. And you want to fix it by making the with-dropindicator height (which is higher than normal) the default height?

I think we should rather stop the drop indicator from influencing the height at all.

Comment 6

9 years ago
i could be wrong, but i'd bet the toolbar is now smaller than before bug 382466 due to the scrollbox instead. the drop indicator before was already part of the toolbar and i suspect changing the hbox to a scrollbox has caused us to lose in height.
Plus the fact this change just unifies the threee platforms, os x is the only one that does not have a min-height but is instead using a bunch of paddings to set the toolbar height. I've also seen some situation (prior to to bug 382466) that were causing the toolbars on os x to flicker in height in some situations.
So yes, i think setting a min-height is the simplest, platforms consistent and better option here.
Btw, i agree that comparing the toolbar heights could be a good way to understand if i'mk correct, so i'll do.
(In reply to comment #6)
> i could be wrong, but i'd bet the toolbar is now smaller than before bug 382466

Doesn't look like that to me. The toolbar height in the current nightly is the same as in Firefox 3.5. Maybe it's a rounding issue?

I agree that adding a min-height is the right way to go, as long as it doesn't change the default toolbar height. :-)

Comment 8

9 years ago
the correct min-height is 23px, not 24px.
the problem is the padding, indeed doing calculation i was getting 24px.
in previous implementation the drop indicator was in a vbox with the toolbar hbox, but its margin was adjusted to a negative value with hacky-magik. the new implementations puts the indicator in the same hbox as the scrollbox, so the padding gets applied to the drop indicator, that is 1px taller.
with a 23px of min-height there are no resizes, and i just compared patched version with a trunk nightly before bug 382466, they have same height.
That explains it. Thanks for looking at this.
So, dupe of that other bug, or are you taking this, Marco?

Comment 11

9 years ago
sorry. taking. It's not a dupe, just that i'm working on the same code and is a regression of the same bug, so i'm just adding the min-height on the other patch.
Adding dependancy.
Assignee: nobody → mak77
Depends on: 512353
Flags: blocking-firefox3.6? → blocking-firefox3.6+

Comment 12

9 years ago
fixed with bug 512353.
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed by bug 512353]
Target Milestone: --- → Firefox 3.7a1


9 years ago
status1.9.2: --- → beta1-fixed
You need to log in before you can comment on or make changes to this bug.