Closed
Bug 325455
Opened 19 years ago
Closed 18 years ago
Drag indicators (tab drag arrow and bookmarks toolbar marker) broken
Categories
(Core Graveyard :: Widget: Mac, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: waynegwoods, Assigned: jaas)
References
Details
Attachments
(3 files, 1 obsolete file)
9.48 KB,
text/plain
|
Details | |
1.38 KB,
patch
|
Details | Diff | Splinter Review | |
1.30 KB,
application/vnd.mozilla.xul+xml
|
Details |
In the Mac trunk nightlies, the tab drag indicator (aka the little purple arrow) has become temperamental.
If you go to move a tab, the arrow now either (a) doesn't appear at all, or (b) appears very briefly, then vanishes. After that, it usually won't appear again while you're still trying to drag the same tab, no matter how much you move the cursor around. I say "usually" here because if the planets are aligned the right way, it'll stay for longer, or even reappear later.
The indicator arrow worked reasonably well in builds up to at least 20051005. The bug appeared by 20051010 at the very latest. I haven't yet had an opportunity to test builds from the four days in between.
Tested in the latest trunk nightly, using a clean profile. No 3rd-party extensions or themes.
Reporter | ||
Comment 1•19 years ago
|
||
Can now confirm that the bug was introduced between the 20051007-06 and 20051008-06 builds. It's still fine in the 1.5 release build, so it looks like the code responsible was never introduced on the branch.
Reporter | ||
Comment 2•19 years ago
|
||
Setting a block request. It should probably be fixed before Firefox 2. It works fine in 1.5, so it'll look a bit poor if users upgrade to Firefox 2 and find it's not working. Though I'll freely admit it's not the most serious of bugs.
Flags: blocking-firefox2?
Comment 3•19 years ago
|
||
Isn't this problem trunk-only? If so, there's no reason it should block Firefox 2...
Reporter | ||
Comment 5•19 years ago
|
||
Duh, you're right. Sorry.
![]() |
||
Comment 6•19 years ago
|
||
OK, I can't seem to reproduce this on Linux... Wayne, do you compile? If so, could you possibly do some testing to see which bug in the checkin window is responsible? Certainly checking whether bug 311550 is responsible should be really easy if you can compile...
Reporter | ||
Comment 7•19 years ago
|
||
(In reply to comment #6)
> OK, I can't seem to reproduce this on Linux... Wayne, do you compile?
I never have, but I'm sure I could (in fact it's about time I probably learned). But I'd need a bit of advice to get started, namely:
What compiler is the code optimized for? gcc?
Whhere would I find the compile options?
Where would I get the source code?
The other problem is I wonder if I would have to set up some sort of cvs, download an enormous volume of code to it, and then do some kind of magic to remove the specific patch? That would be difficult for me, as I'm still only on dial-up at home. I can download all I like from work (in Windows), but if I had to download directly to a cvs on my home Mac, I expect I wouldn't have that option.
If you're able to point me to a page where I could find some help, or are able to offer any advice yourself, it'd be much appreciated. And I could then use the skills to help diagnose other bugs, as well. Feel free to email me directly, if you want.
Thanks!
![]() |
||
Comment 8•19 years ago
|
||
http://developer.mozilla.org/en/docs/Build_Documentation has directions for building on our main platforms (including Mac OS X).
It looks like we no longer do nightly source tarballs, so you'd have to either download the 1.5 tarball and then CVS update it to tip, or just check out tip directly from CVS. Either one would be a little painful over dialup. :(
Josh, can you reproduce this, by any chance?
Reporter | ||
Comment 9•19 years ago
|
||
OK, with a few hiccups, I worked out how to run a cvs and compile Firefox at home :)
When I build the latest trunk, there's an oddity, however... it creates an app of only about 7.6 MB, instead of the ~28.8 MB of a trunk download from mozilla.org. However, the folder in which it's created becomes filled with other files and folders, which I presume are being used by the app and should normally be placed IN the application bundle... as if its guts had been pulled out. So it looks like I still have some work to do to get it right.
However... the app DOES load and work correctly (presumably it can reference the externalized files). As expected, it still exhibits the broken tab drag indicator arrow.
Now, if I manually edit view/src/nsViewManager.cpp to comment out the code you added in bug 311550 (attachment 198822 [details] [diff] [review]), and then recompile, the bug goes away and the arrow works correctly again. So it definitely looks like your fix is causing this bug, or perhaps exposing an underlying problem.
As an aside... before recompiling, I renamed the original app bundle and kept it (but not the externalized files). After the recompile, the bug was gone from that version as well. I presume the recompile changed something in one of the externalized files that the app references. Not sure. After deleting everything and compiling the proper trunk code again, the bug returned as expected.
![]() |
||
Comment 10•19 years ago
|
||
haaara, mento, could you check this out in Firefox? Comment out the early return, then find where we're hitting the mHasPendingUpdates == 0 case? In particular, what aUpdateFlags are and where we're being called from?
My best guess is that _something_ either depends on previous invalidate events to work in a timely manner or just fails to set mHasPendingUpdates when it has some (so depends on other invalidates to work at all).
Blocks: 311550
Flags: blocking1.9a1?
Comment 11•19 years ago
|
||
In cocoa widgets, dragging tabs seems to not work *at all*. The drag is rejected, basically.
I'm building a carbon tree now, and will have a look from both sides.
![]() |
||
Comment 12•19 years ago
|
||
Yeah, this bug is about carbon. Let's not even go cocoa-ways. ;)
Comment 13•19 years ago
|
||
I commented out the early return, set a conditional breakpoint there, and printed a little stack context (and the update flags).
While dragging, we seem to get both NS_VMREFRESH_DEFERRED and NS_VMREFRESH_IMMEDIATE, where we currently return.
The stacks says it's both from CSS code, and from ESM (drag/drop events).
I'll leave out the analysis of this data to the layout gurus. Note that commenting out the early return does not fix the bug (entirely)...
Reporter | ||
Comment 14•19 years ago
|
||
Interesting that it didn't entirely fix the bug for you.
Is it possible that some of what you're still seeing is part of bug 333791? The other bug is cross-platform, and seems to have existed for as long as the tab drag arrow has. I only just filed it though... I'd been hoping to get this one cleared up before I added to the confusion ;)
![]() |
||
Comment 15•19 years ago
|
||
OK. Next step: do the early return only if mHasPendingUpdates == 0 and aUpdateFlags is 1? Then the same thing for 2? Do both fix this bug? Or just one of them?
Comment 16•19 years ago
|
||
(In reply to comment #14)
> Is it possible that some of what you're still seeing is part of bug 333791? The
> other bug is cross-platform, and seems to have existed for as long as the tab
> drag arrow has. I only just filed it though... I'd been hoping to get this one
> cleared up before I added to the confusion ;)
Well, it could be. I think these bugs are very similar...
Comment 17•19 years ago
|
||
OK, here's some more testing results.
I've now compared with Firefox 1.5 behavior to ensure this bug is about fixing the regression (so we go back to the old buggy behavior :P).
* If you return when aUpdateFlags == 2, no arrow is shown
* If you return when aUpdateFlags == 1, the old behavior is resurrected
So the conclusion seems to be that we must not optimize away the refreshes when aUpdateFlags is 2, for mac dragging to work properly.
That might or might not have to do with our buggy dragging service, that will be rewritten sometime in the upcoming bright cocoa future.
![]() |
||
Comment 18•19 years ago
|
||
> * If you return when aUpdateFlags == 2, no arrow is shown
This is the NS_VMREFRESH_IMMEDIATE case. Can you step into nsViewManager::FlushPendingInvalidates in this case? When we get down to the check for mHasPendingUpdates, is it still 0? I'd expect that it is...
![]() |
||
Comment 19•19 years ago
|
||
Does removing the IsDragInProgress() check at the end of PresShell::AppendReflowCommand help here?
Comment 20•19 years ago
|
||
(In reply to comment #19)
> Does removing the IsDragInProgress() check at the end of
> PresShell::AppendReflowCommand help here?
It didn't make any to me noticable change.
I think we should check in the commented out early return (at least in the NS_VMREFRESH_IMMEDIATE case), as that seems to give us the same semi-buggy (sometimes flickery, albeit functional) behavior.
Comment 21•19 years ago
|
||
I wanted to say: if we comment out that line, it gives us parity with other platforms.
![]() |
||
Comment 22•19 years ago
|
||
Um. How about we find the real problem instead? Maybe if we do we can remove the ESM hacks that try to force repaint during dragging too.
If you could do what I asked for in comment 18, that would be great.
Comment 23•19 years ago
|
||
(In reply to comment #18)
> > * If you return when aUpdateFlags == 2, no arrow is shown
>
> This is the NS_VMREFRESH_IMMEDIATE case. Can you step into
> nsViewManager::FlushPendingInvalidates in this case? When we get down to the
> check for mHasPendingUpdates, is it still 0? I'd expect that it is...
Ok, I did this:
* Comment out the early return
* When aUpdateFlags & NS_VMREFRESH_IMMEDIATE I "broke" into the debugger
* Stepped into FlushPendingInvalidates() a little further down
In every drag case I was able to do (you'll break into the debugger immediately basically) the mHasPendingUpdates was still 0. This was when the ESM path was executed.
![]() |
||
Comment 24•19 years ago
|
||
OK. Can you change the early-return thing so that when mHasPendingInvalidates is 0 and aUpdateFlags & NS_VMREFRESH_IMMEDIATE we skip the FlushPendingInvalidates() call but do call Composite()? Does that help?
Comment 25•19 years ago
|
||
Yup, that also fixes it, like when you comment out the early return.
The only remaining issue then is that the arrow flickers slightly when you drag near the favicon or the edges. I believe that is actually a separate issue, see the last few comments on bug 333791.
![]() |
||
Comment 26•19 years ago
|
||
So it looks like Mac does SOMETHING that doesn't actually invalidate but expects to be repainted. I'd like to know what and why. The only reason this ever worked is the "immediate repaint" hack during drag-drop in the ESM.
I'm really not sure how to hunt for code that doesn't do something. :(
Assignee: nobody → joshmoz
Component: Tabbed Browser → Widget: Mac
Product: Firefox → Core
QA Contact: tabbed.browser → mac
Summary: [Mac] Tab drag indicator (arrow) broken → Tab drag indicator (arrow) broken
![]() |
||
Comment 27•19 years ago
|
||
I'm also somewhat tempted to see whether with cocoa widgets the problem goes away. ;)
Comment 28•19 years ago
|
||
(In reply to comment #27)
> I'm also somewhat tempted to see whether with cocoa widgets the problem goes
> away. ;)
It hasn't (yet), but who knows what rewrites Josh has up his sleeves. ;)
Reporter | ||
Comment 29•19 years ago
|
||
(In reply to comment #26)
> So it looks like Mac does SOMETHING that doesn't actually invalidate but
> expects to be repainted. I'd like to know what and why.
When you mouseover a backgrounded tab, it temporarily sharpens it up to make it looks like a foregrounded tab. Is that a possible candidate for what you describe?
![]() |
||
Comment 30•19 years ago
|
||
Unlikely, since what's not painting is the arrow. As in, whatever is done to move the arrow for some reason doesn't invalidate things right. Or something.
Reporter | ||
Comment 31•19 years ago
|
||
There's a minor UI peculiarity that may, or may not, help with this:
With multiple tabs open, drag one on top of another with a single, swift motion. Don't hesitate at all or move the tab again. Keep the mouse pressed. Then, after 1-4 seconds, move the mouse slightly. You'll notice a tool tip suddenly appear that displays the name of the tab that you're moving. This is the same tool tip that would have appeared if you'd left the mouse hovered over the tab you just moved. Note that if you hesitate more than about 5.5 seconds before moving the second time, the tool tip won't appear. This happens to be the same length of time that the tool tip remains open normally, so I assume it's just that. This oddness seems to have existed for a very long time... long before this bug.
Anyway, the thing is that if you reproduce that weirdness in a build that has the current bug, then the tab drag indicator will also appear if/when the tool tip does. That's the only way I can force it to appear, and can only do it once per drag attempt.
Maybe that info doesn't help, however... maybe the drawing of the tool tip just forces some sort of repaint that circumvents this bug.
Reporter | ||
Comment 32•19 years ago
|
||
Perhaps more important to note, this bug identically affects the drag indicator that appears when you try to drag a tab onto the bookmarks toolbar. That's the indicator that looks like a purple vertical bar. The phenomenon mentioned in comment 31 affects that indicator the same way. Similarly, that indicator freezes, or fails to appear, when you try to move bookmarks buttons around on the toolbar, or try to drag a url from the location bar to the toolbar.
One further peculiarity with this is that if you mouse over a popup menu (bookmark submenu) while dragging a tab or button over the toolbar, then the indicator will redraw, once. I guess something about the popup forces a repaint. This doesn't seem to happen if you're dragging a favicon/url over the toolbar, however. Not sure what the difference is.
Summary: Tab drag indicator (arrow) broken → Drag indicators (tab drag arrow and bookmarks toolbar marker) broken
![]() |
||
Comment 33•19 years ago
|
||
Reporter | ||
Comment 34•19 years ago
|
||
Unfortunately that doesn't fix it.
Reporter | ||
Comment 36•19 years ago
|
||
That didn't seem to fix it, either.
![]() |
||
Comment 37•19 years ago
|
||
OK.... Can someone test whether mHasPendingUpdates is ever nonzero in EnableRefresh while dragging the tabs on Mac?
Comment 38•19 years ago
|
||
(In reply to comment #37)
> OK.... Can someone test whether mHasPendingUpdates is ever nonzero in
> EnableRefresh while dragging the tabs on Mac?
>
Is that with or without any of the patches applied?
![]() |
||
Comment 39•19 years ago
|
||
Without any patches.
Comment 40•19 years ago
|
||
(In reply to comment #37)
> OK.... Can someone test whether mHasPendingUpdates is ever nonzero in
> EnableRefresh while dragging the tabs on Mac?
Yes, it's 0 and sometimes 2.
![]() |
||
Comment 41•19 years ago
|
||
Er... mHasPendingUpdates is a PRBool. It can't be "2". Are we talking about the same variable here? ;)
Comment 42•19 years ago
|
||
(In reply to comment #37)
> OK.... Can someone test whether mHasPendingUpdates is ever nonzero in
> EnableRefresh while dragging the tabs on Mac?
Obviously, I printf()'d the wrong variable earlier. But I noticed that I put the printf() statement *after* the early return if (!mHasPendingUpdates).
So, the answer to your question is: yes, it's non-zero in enable refresh when dragging tabs, and the data I accidently wrote here is what aUpdateFlags was. :-)
![]() |
||
Comment 43•19 years ago
|
||
I see.... Can you debug to see why the arrow is not painting, then? What do the display lists look like? See gDumpPaintList in nsLayoutUtils.cpp.
Also, this is starting to sound like something that would really benefit from a minimal testcase... Can someone familiar with the firefox arrow XUL make one up?
Reporter | ||
Comment 44•19 years ago
|
||
Interesting. A change occurred between the 20060510-06 and 200605011-06 builds that has "mostly fixed" the tab drag indicator problem.
Now, the first time you move a tab (in a new window), the indicator still fails to appear. Subsequent times, it does work. However lack of initial appearance is probably due to a new regression, as it's recently been reported for Windows: bug 338302.
There's also still the issue of the indicator vanishing when passing over buttons and icons on the tab, but that's bug 333791.
However, one other problem still remains. If you move the cursor outside the tab bar, the arrow vanishes (probably expected behaviour). But if you move it back, the arrow won't reappear until you drag over the mid-way point of a tab. It should appear immediately at the closest tab junction. It looks like the mid-way point is the only trigger to draw the arrow. It should be triggered upon initial drag over. I wonder if that's still this bug, or if it should be filed separately... I won't have a chance to see if it's doing the same thing on Windows for a couple of days.
I wonder if it was the check-in of the new thread manager that has mostly fixed the problem? (bug 326273). The changes with that are too vast for me to consider reversing them to see.
![]() |
||
Comment 45•19 years ago
|
||
> I wonder if it was the check-in of the new thread manager
That's the only checkin between the two nightly builds you cite; we did that on purpose, for regression-finding purposes...
So yeah, sounds like it helped.
As for the midpoint of the tab thing, does backing out the patch from bug 311550 help with that? If not, doesn't sound like it's really this bug, since this bug has been all about the issue caused by bug 311550.
Reporter | ||
Comment 46•19 years ago
|
||
Good point. And yes, backing out the patch from bug 311550 *does* remove that particular issue (but obviously not the issue mentioned in bug 338302). So this bug is still relevant.
Basically, what now remains of this bug is that the tab drag indicator arrow fails to be drawn *except* when the mid-point of the tab is dragged over (and on very rare occasions when the edge of tab is hit). If an arrow is present, and you drag the cursor to outside the tab, or if you hit one of the "flicker" regions mentioned in bug 333791, then the arrow vanishes entirely, and won't come back until you pass over a tab mid-point again.
I should also mention that the drag indicator (purple bar) on the bookmarks toolbar seems to work properly now. Tested by dragging a tab, a location bar favicon, and another bookmark toolbar button. With all three, the indicator seems to work correctly.
Reporter | ||
Comment 47•19 years ago
|
||
Just noting that the patch over on bug 333791 seems to fix the remaining issues here (or at least treats the symptoms... I'm not sure if there are underlying Mac issues that remain). That patch is only a first draft, though, and not complete.
![]() |
||
Comment 48•19 years ago
|
||
So what does that patch actually change?
Reporter | ||
Comment 49•19 years ago
|
||
It's an adaptation of Jag's patch for SeaMonkey, to remove the problem where the arrow flickers or vanishes when mousing over tab elements. Mostly it involves more intelligent hiding of the indicator by onDragExit, which previously just aways hid it. I'm no genius at adapting the code, and I'm still learning what things do, but it works (attachment 223921 [details] [diff] [review]).
The only problem is the indicator arrow then refuses to hide when you drag over the content window. I've tinkered with various things, but haven't had the time to work out why yet. I suspect the answer lies in bug 333791 comment 11.
![]() |
||
Comment 50•19 years ago
|
||
So we used to hide the arrow and then reshow it and on mac the reshowing is broken (this bug)?
Reporter | ||
Comment 51•19 years ago
|
||
(In reply to comment #50)
> So we used to hide the arrow and then reshow it and on mac the reshowing is
> broken (this bug)?
Actually, that patch seems to help with the reshowing as well. When I posted comment 47, I thought it fixed the reshowing completely. But now that I've tested it again, there seems to be times when the arrow is hidden, and you drag over a tab and it still remains hidden. Yet later you do exactly the same thing and it'll appear. I'm not sure what's going on there.
What's the best way to add debug reports to the code? It'd help immensely if I could send myself messages (or values of variables) as functions are called and executed. I've tried a number of things, but so far all of them just seem to break the code. :(
![]() |
||
Comment 52•19 years ago
|
||
> What's the best way to add debug reports to the code?
printf?
Reporter | ||
Comment 53•19 years ago
|
||
(In reply to comment #52)
> > What's the best way to add debug reports to the code?
> printf?
That's actually the first function I tried, but it just seems to break things.
For example, if I insert a simple printf message at the start of the onDragOver method:
<method name="onDragOver">
<parameter name="aEvent"/>
<parameter name="aFlavour"/>
<parameter name="aDragSession"/>
<body>
<![CDATA[
printf("onDragOver CALLED!\n");
Tabs can no longer be dropped, and the drag indicator vanishes. I presume printf() is causing a silent error.
If it did work, I'm not sure where it'd write to, anyway. I've tried checking the Error Console, the Apple Console logs, and running the app from the Terminal command line. Perhaps it can't find a stdout.
![]() |
||
Comment 54•19 years ago
|
||
Oh. You're talking about JS. use dump(), and make sure you set the relevant debug pref (name contains "dump_enabled" or something).
Reporter | ||
Comment 55•19 years ago
|
||
(In reply to comment #54)
> Oh. You're talking about JS. use dump(), and make sure you set the relevant
> debug pref (name contains "dump_enabled" or something).
Excellent. Thanks!
I added a debug message to start of the methods "onDragStart", "onDragOver" and "onDragExit".
If I click on a tab in roughly the middle, and then start dragging a tab *slowly* towards the contents of the page, I get, in order:
onDragStart CALLED
onDragExit CALLED (not sure why... the mouse has barely moved at this point!)
onDragOver CALLED (3x in a row)
onDragExit CALLED (I think this is when the mouse hits the narrow strip at the bottom of the tab bar)
onDragOver CALLED (4x)
onDragExit CALLED (I think when the mouse hits the content window)
onDragOver CALLED (3x)
However, if I do the same drag, but *quickly*, I just get:
onDragStart CALLED
onDragOver CALLED
So onDragExit isn't even executed once in that case. Is that expected behaviour?
![]() |
||
Comment 56•19 years ago
|
||
No idea. I kinda try to ignore our drag/drop code as much as I can....
Neil, do you know anything about this stuff?
Comment 57•19 years ago
|
||
Not a lot... lack of a drag exit sounds like a widget bug, although in fact it's the drag over that actually causes the arrow to show. What happens if you slowly drag the tab across until the arrow shows, then quickly drag down?
Reporter | ||
Comment 58•19 years ago
|
||
(In reply to comment #57)
> What happens if you
> slowly drag the tab across until the arrow shows, then quickly drag down?
If the arrow is already showing, I find that onDragExit is much more likely to be called, so it's harder to reproduce. But if I drag quickly enough, I can still generate a situation where it's not called. When that happens, the arrow doesn't vanish, but sticks where it was left last. At least until the tab is dragged over again. dragExit(), which calls onDragExit, is likewise not called in that situation, so the bug lies earlier than that. dragExit seems to be referenced in a property of the tabbrowser-strip hbox as ondragexit="nsDragAndDrop.dragExit(event, this.parentNode.parentNode); event.stopPropagation();"
How ondragexit is triggered, I don't know.
Comment 59•19 years ago
|
||
(In reply to comment #58)
>How ondragexit is triggered, I don't know.
There appear to be two cases. One case is when you drag within the window. The event state manager tracks the last dragged element so if it's about to change it switch element it sends an exit to the old element.
The other case is when you drag out of the window. In this case the widget code explicitly sends a drag exit event. I'm not sure, but it might also apply when dragging from chrome to content.
Note: I can't comment on the Mac widget code in any way.
Reporter | ||
Comment 60•19 years ago
|
||
The issue with this bug now seems to be that setting ind.style.marginLeft (or ind.style.marginRight) is supposed to always trigger the indicator arrow to redraw.
However, on Mac it's only doing so if the new marginLeft is different to the previous marginLeft. Hence the reason that the arrow only appears after you cross the "mid-line" of the tab, but not if you cross between one tab and the next (as the marginLeft stays the same in the latter case). On Windows, I'm guessing it must draw regardless of whether the indicator changes position or not.
To test that on Mac, I generated a browser in which the marginLeft was set slightly differently if the drag was in the tab to the left of the indicator. In that case, the arrow then appeared when crossing from one tab to another.
Hope that helps to track it down.
(Oh, and the phenomenon I reported in comment 55 is apparently normal, and unrelated to this bug... see 342361 comment 16, which was a reply to 342361 comment 12)
![]() |
||
Comment 61•19 years ago
|
||
> However, on Mac it's only doing so if the new marginLeft is different to the
> previous marginLeft.
That's the right behavior... if the marginLeft hasn't changed, why would a redraw be needed? That is, the setting of a margin value to its existing value should not cause the invalidate. Whatever causes the movement of the arrow should cause the invalidate. If the margin value is not changing, why is the arrow moving?
Reporter | ||
Comment 62•19 years ago
|
||
Sorry, bad guess. I assume the arrow really is supposed to appear as soon as dragging is set to true and not when its position is set.
'dragging' is set as true on the tab-drop-indicator-bar, and its display is set to -moz-box in the css, which is presumably then supposed to affect the display of its children (i.e. tab-drop-indicator). In that situation, is it possible that the child is supposed to be automatically invalidated as part of the display change of the parent, but it isn't being done on Mac?
![]() |
||
Comment 63•19 years ago
|
||
Changing the display type will cause frame construction, which will cause reflow. The reflow is supposed to invalidate the relevant areas. That code should be cross-platform. Now it's possible that it's broken cross-platform and that on non-mac OSes something else invalidates things... roc, do you have time to check on invalidation in XUL reflow?
Reporter | ||
Comment 64•19 years ago
|
||
Maybe there is other Windows code that compensates for a hidden cross-platform problem... My original suspicion(presumably incorrect) that Windows redraws objects set to the same coordinates while Mac OS X doesn't was based on a comment in the window-moving code:
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacWindow.cpp#1228
Probably completely unrelated, but I thought it was worth mentioning.
But if this effect really is cross-platform, then there's a second cross-platform bug to do with things not repainting right in the vicinity of tabs: bug 328193, which is about bad outlines and tab elements. But I have no idea if the second bug is due to problems with reflow.
![]() |
||
Comment 65•19 years ago
|
||
Frankly, what this bug could really really use is a smaller testcase than the browser UI. Preferably a testcase that doesn't depend on drag&drop and instead just manipulates stuff in JS (though this may be impossible if the drag/drop event handling is relevant here).
Reporter | ||
Comment 66•19 years ago
|
||
Sorry, Boris, I haven't worked out how to generate a suitable xul testcase that can be loaded within a browser window.
However, I think I may have narrowed it down heaps... Please see if this makes sense to you...
If I gut onDragOver and reduce it to the following lines:
if (aDragSession.sourceNode) {
var ib = this.mTabDropIndicatorBar;
var ind = ib.firstChild;
ib.setAttribute('dragging', 'true');
ind.style.marginLeft = '200px';
}
Then the bug goes away... the indicator is ALWAYS drawn when it should be. Even after it's hidden by, say, dragging over the content window and then revealed by dragging back over the tab bar. And even though it's being drawn in exactly the same place each time, which is what triggers the bug normally.
However, if I add the following innocent-seeming code after dragging is set to true:
ib.boxObject.x;
Then the bug returns in full-force... the indicator is NEVER drawn.
|ib.boxObject.x| is normally called twice in the code... during calculation of the max and min MarginLeft values. It's not that the code is breaking, as a dump() reveals that it's returning correct values. But it looks to me that even one call is enough to prevent the indicator being (re)drawn.
A use of |ib.boxObject.width| has the same effect, so it's not just .x. But if I just reference |ib.boxObject| then the bug goes away.
Does this make sense to you?
![]() |
||
Comment 67•19 years ago
|
||
> ib.setAttribute('dragging', 'true');
So this triggers the following CSS rule in browser.css:
65 .tab-drop-indicator-bar[dragging="true"] {
66 display: -moz-box;
67 }
which creates a frame for "ib".
> ind.style.marginLeft = '200px';
And this sets style on a child of "ib".
But the frame construction is lazy, so the frame is only constructed after this function returns, at which point the left margin is 200px, and all is good.
> ib.boxObject.x;
This flushes out frame construction and reflow. So once you run this, all pending reflows and restyles are processed. If you set the margin-left after this, it will be processed all on its own.
So all that is consistent with there not being an invalidate when that margin-left changes...
One thing I'm kinda curious about -- would you be able to do a reflow branch build (directions at http://wiki.mozilla.org/Gecko:Reflow_Refactoring#Build_Instructions) and see whether this is still a problem there? It's a bit of a hassle, so I can understand if you don't really want to do that. Just let me know, ok?
![]() |
||
Comment 68•19 years ago
|
||
Do you see a problem with this file (esp. when loaded as -chrome)?
Reporter | ||
Comment 69•19 years ago
|
||
(In reply to comment #68) > Created an attachment (id=232470) [edit] > Possible testcase? > > Do you see a problem with this file (esp. when loaded as -chrome)? I only loaded it in a browser tab. I'm not sure how to "load as -chrome" :) Anyway, when I load the page (or force a no-cache reload), the first 1-3 times I enter the green box, the image (which looks like a big "[") fails to appear... but maybe that's a separate problem. After that, it appears every time. It's situated off to the right, outside the box. If I edit the test() func so that marginLeft isn't changed in the second condition (which makes it more like the real situation), the image still appears every time.
Reporter | ||
Comment 70•19 years ago
|
||
(In reply to comment #67) > One thing I'm kinda curious about -- would you be able to do a reflow branch > build and see > whether this is still a problem there? It's a bit of a hassle, so I can > understand if you don't really want to do that. Just let me know, ok? I'll see what I can do, but I can't guarantee anything in a hurry as I haven't even worked out how to maintain multiple branches in my cvs yet. I thought I was doing well just working out how to maintain the trunk repository at home, over my slow dial-up ;) You don't know any Mac people involved in the reflow work that could spot me one of their builds instead? :)
![]() |
||
Comment 71•19 years ago
|
||
> I'm not sure how to "load as -chrome"
firefox -chrome URL
And there are only two people doing reflow branch stuff; neither one of us is using a Mac... I wouldn't worry about the reflow branch for the time being.
roc, do you have any idea what's up here? It really does sound like some part of XUL layout doesn't invalidate things correctly or something...
Reporter | ||
Comment 72•19 years ago
|
||
(In reply to comment #71)
> firefox -chrome URL
Thanks. When I do this, Minefield appears as a very tiny window that encompasses ONLY the green box. So I can't help but move the mouse outside the window. The '[' image never appears.
![]() |
||
Comment 73•19 years ago
|
||
er... yeah. You'd need to add a default size to the <window> that's big enough to see the whole thing. :(
Reporter | ||
Comment 74•19 years ago
|
||
Thanks. I set it to maximized. The hovering behaved exactly the same way in chrome as when I opened it within a tab, i.e. didn't work right for the first time or 3, but then worked every time after that. After it worked once, it worked every time I restarted the browser. Presumably because of caching.
nsBox::SyncLayout gets called after any box frame is reflowed, and if the box frame was dirty it calls nsBox::Redraw(), which invalidates the overflow area of the frame. I guess that is supposed to be enough.
Have you got a tool called quartzdebug? That can show you which areas are being repainted by the application. Go through the testcase and note which areas are repainted at each step.
Reporter | ||
Comment 76•19 years ago
|
||
Actually, one thing I was wrong about Boris's testcase... that image IS painted even in the first few hovers, but it's delayed by several seconds. I guess I was too impatient and didn't see that. On all subsequent occasions, it's drawn immediately. What could cause that initial delay? Quartzdebug highlights everything as it's drawn. But since everything IS drawn (albeit slowly the first time), I'm not sure what useful info it might be giving me for that testcase. I tried running Quartzdebug while dragging tabs. If the dnd indicator changes location, then a strip that encompasses the lower half of the bookmarks bar, and the very top section of the tab bar is redrawn (as well as the indicator arrow). If I drag between tabs nothing is repainted, as the indicator doesn't change positions in that situation. If I drag over the content area, then the area that is repainted encompasses the bottom half of the bookmarks bar, all of the tab bar, and the entire content window. If I re-enter the tab bar at a position that requires the drag indicator to reappear in a different location, then the same region (from bookmarks bar to content window) is repainted. If I re-enter at a position where the indicator arrow should reappear in its old location, then NOTHING is repainted (except for the drag outline area). I don't know if that's helpful or not, but I'm not sure what else I should be looking for.
![]() |
||
Comment 77•19 years ago
|
||
> What could cause that initial delay?
Having to load the image from the web.
Reporter | ||
Comment 78•19 years ago
|
||
With the new tab scrolling code, I'm seeing a similar problem when dragging a tab and scrolling at the same time... see bug 348526. I presume it's due to the same problems as this bug.
Updated•18 years ago
|
Flags: blocking1.9a1? → blocking1.9+
Reporter | ||
Comment 79•18 years ago
|
||
This bug seems to have been vanished since d&d was fixed in the Cocoa builds.
You haven't landed any of the reflow branch stuff on the trunk yet, have you, Boris? I presume it's just the Cocoa thing that fixed it.
I'll set as WORKSFORME, anyway.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
![]() |
||
Comment 80•18 years ago
|
||
No, there's been no reflow branch landing.
Good to know that widget work fixed this!
You need to log in
before you can comment on or make changes to this bug.
Description
•