Closed
Bug 508499
Opened 15 years ago
Closed 15 years ago
simplify tab drop indicator code and styling
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
FIXED
Firefox 3.7a3
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #392672 -
Attachment is obsolete: true
Attachment #392722 -
Flags: review?(rflint)
Assignee | ||
Comment 2•15 years ago
|
||
updated to trunk
Attachment #392722 -
Attachment is obsolete: true
Attachment #406659 -
Flags: review?(rflint)
Attachment #392722 -
Flags: review?(rflint)
Assignee | ||
Updated•15 years ago
|
Attachment #406659 -
Flags: review?(rflint) → review?(mano)
Assignee | ||
Updated•15 years ago
|
Attachment #406659 -
Flags: review?(mano) → review?(mconnor)
Comment 3•15 years ago
|
||
Comment on attachment 406659 [details] [diff] [review]
patch
I am no longer actively working on Firefox, please find another reviewer.
Attachment #406659 -
Flags: review?(mconnor)
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #406659 -
Attachment is obsolete: true
Attachment #426891 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 5•15 years ago
|
||
Attachment #426891 -
Attachment is obsolete: true
Attachment #428213 -
Flags: review?(gavin.sharp)
Attachment #426891 -
Flags: review?(gavin.sharp)
Comment 6•15 years ago
|
||
Comment on attachment 428213 [details] [diff] [review]
patch
This patch seems to be causing the dropmarker to appear unreliably. I tested on Mac and Linux. I can't make any sense of it, it seems like a strange layout-related issue - dumps in _onDragOver show that we're setting the margin correctly and uncollapsing the indicator, but it just doesn't actually appear in a lot of cases. I can usually get it to appear by dragging out of the tabstrip and then back in (which collapses in _dragOut and then uncollapses again in _onDragOver). Forcing a reflow at the end of onDragOver doesn't seem to have an effect.
My STR are to select a tab near the end of the tabstrip and drag it back and forth towards the beginning of the tabstrip. The dropmarker only appears sporadically. Often it will appear while dragging back towards the original location, but not when dragging away from it, which doesn't really make any sense to me. Reverting the patch makes it appear consistently again.
Assignee | ||
Comment 7•15 years ago
|
||
Maybe a regression from bug 526394, like bug 545049...
Comment 8•15 years ago
|
||
(In reply to comment #7)
> Maybe a regression from bug 526394, like bug 545049...
if it's a scrollbox, that's probably more than a maybe.
Assignee | ||
Comment 9•15 years ago
|
||
I would think it's position: relative; z-index: 1; rather than the scrollbox, but that's only a guess. Either way both drop indicators have a lot in common, and either way the underlying problem needs to be fixed, since it did work flawlessly when I wrote the patch.
Assignee | ||
Comment 10•15 years ago
|
||
I just tested the patch with mozilla-central from January 10 (worked) and from January 12 (failed), so this is very likely bug 545049.
Depends on: 545049
Assignee | ||
Comment 11•15 years ago
|
||
updated to tip
Can we land this, knowing that it suffers from bug 545049 just like the bookmarks toolbar?
Attachment #428213 -
Attachment is obsolete: true
Attachment #429969 -
Flags: review?(gavin.sharp)
Attachment #428213 -
Flags: review?(gavin.sharp)
Comment 12•15 years ago
|
||
Comment on attachment 429969 [details] [diff] [review]
patch
I guess so. Maybe file a separate bug and mark it dependent, in case the fix for that bug doesn't also fix this one? Granted that's unlikely, but better safe than sorry I think.
Attachment #429969 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 13•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/3b49923ba8c5
Roc just took bug 545049, which I'll monitor closely. Will file a separate bug if necessary.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a3
Comment 14•15 years ago
|
||
this also fix the problem
+ var ib = ind.parentNode;
- ind.style.MozTransform = "translate(" + Math.round(newMargin) + "px)";
+ ib.style.MozTransform = "translate(" + Math.round(newMargin) + "px)";
ind.style.MozMarginStart = (-ind.clientWidth) + "px";
- ind.style.marginTop = (-ind.clientHeight) + "px";
+ ib.style.marginTop = (-ind.clientHeight) + "px";
You need to log in
before you can comment on or make changes to this bug.
Description
•