when dragging to reorder a tab, scrolling stops in certain scenario here's my scenario: 1) set the min tab width pref to a large value, to make this easier to reproduce. 2) open many tabs 3) select one in the middle and drag it to the left 4) drag until scrolling start 5) stop dragging when the original tab is "cut off" half way 6) without letting go of the tab, attempt to drag back to the right actual results: you can't drag the tab to the right (and I get the "no drop" cursor) desired result; you are able to drag the tab to the right I fear this might be related to the "hide the arrow when dragging on/under the active tab" of bug #333791 I'm going to revert the change to "can drop" to confirm this is the cause of the regression.

> I fear this might be related to the "hide the arrow when dragging on/under the > active tab" of bug #333791 > > I'm going to revert the change to "can drop" to confirm this is the cause of > the regression. yes, when I remove the newly added canDrop() from tabbrowser.xml which was added for the "hide the arrow when dragging on/under the active tab" part bug #333791, this regression goes away. I think this regression is worse than not hiding the drop indicator when dragging over the active tab. should this block ff2?

I think that a way to fix this is to change how we use canDrop in onDragOver. instead of returning right away if !canDrop, we could do this: ib.setAttribute('dragging', aDragSession.canDrop ? 'true' : 'false'); testing it out...

about my scenario, it is even easier to reproduce this regression. drag scrolling will halt as soon as you hit the selected tab. but I have a fix to onDragOver to prevent scrolling from stopping, but, the side effect is that the drag indicator will disappear when scrolling when you are over the selected tab. (maybe that is ok or even desirable, as that is what part of bug #333791 was about: not showing the indicator when over the currently selected tab) here comes a patch...

Sorry, my bad. I tested the canDrop code out with tab scrolling, but didn't encounter that problem. Unfortunately, it's hard to detect a new tab scrolling bug on Mac because it's still so horribly jerky anyway... any interruptions in scrolling seem to be "normal". The fix here seems to work fine for me. I wonder if it'd be better to put a check in canDrop to return true if the drag is over an active tab scroller, and then ignore the tab underneath it entirely. I think it's OK for the indicator to be visible if you're over the scroller, even if the active tab is underneath. But then maybe a canDrop check like that is too costly?

Comment on attachment 231146 [details] [diff] [review] patch Ok, let's get this in ASAP

fixed

> Sorry, my bad. I tested the canDrop code out with tab scrolling, but didn't > encounter that problem. no worries. I didn't catch it when I reviewed or tested either. > Unfortunately, it's hard to detect a new tab scrolling > bug on Mac because it's still so horribly jerky anyway... any interruptions in > scrolling seem to be "normal". I haven't noticed it being really jerky on the mac. can you start a bug on that? > The fix here seems to work fine for me. I wonder if it'd be better to put a > check in canDrop to return true if the drag is over an active tab scroller, > and then ignore the tab underneath it entirely. I think it's OK for the > indicator to be visible if you're over the scroller, even if the active tab > is underneath. But then maybe a canDrop check like that is too costly? see bug #342363 and #342365, especially beltzner's comments. from his comments, and your comments in this bug, how about this: while dragging and scrolling, the drop indicator should be at a 45 degree angle (so we'd need two new images) in the direction of the scroll, and should always appear. when at the edge (and scrolling has stopped) we'd continue to use the 45 degree angle indicator. if not at the edge, and not scrolling (but still dragging), we'd use the current drop indicator. if that seems reasonable, i'll spin up a new bug on that (or morph one of the existing bugs.)

(In reply to comment #8) > from his comments, and your comments in this bug, how about this: > while dragging and scrolling, the drop indicator should be at a 45 degree angle > (so we'd need two new images) in the direction of the scroll, and should always > appear. when at the edge (and scrolling has stopped) we'd continue to use the > 45 degree angle indicator. if not at the edge, and not scrolling (but still > dragging), we'd use the current drop indicator. The angled arrow effect sounds nice. However, in my previous comment, I was just (not very effectively) suggesting that maybe there's no need to set 'dragging' as false when scrolling when the active tab happens to pass underneath the scroll arrow. You'll notice during drag-scrolling that the arrow briefly vanishes as that tab sails by, but I think in that context the effect really doesn't add anything other that to appear as a flicker. If so, and the drag is over a scroll arrow, I suspect you could simply always return true in canDrop, and completely ignore the tab that is underneath it, rather than check all that in onDragOver.

> However, in my previous comment, I was just (not very effectively) suggesting > that maybe there's no need to set 'dragging' as false when scrolling when the > active tab happens to pass underneath the scroll arrow. You'll notice during > drag-scrolling that the arrow briefly vanishes as that tab sails by, but I > think in that context the effect really doesn't add anything other that to > appear as a flicker. If so, and the drag is over a scroll arrow, I suspect you > could simply always return true in canDrop, and completely ignore the tab that > is underneath it, rather than check all that in onDragOver. ah, now I understand. thanks for clarifying. I think this would do what you are suggesting: ib.setAttribute('dragging', (aDragSession.canDrop || pixelsToScroll) ? 'true' : 'false'); as pixelsToScroll will have a non-zero value when the original target of the onDragOver event is the scrollbutton-down or scrollbutton-up. let me spin off that change ("should drop indicator always be shown when drag scrolling?") so that I can get beltzner/ben/mconnor to comment on the UI decision, and keep this bug about fixing the regression.

fixed on branch. > let me spin off that change ("should drop indicator always be shown when drag > scrolling?") so that I can get beltzner/ben/mconnor to comment on the UI > decision, and keep this bug about fixing the regression. I've logged bug #346704 on this issue.

(In reply to comment #8) > I haven't noticed it being really jerky on the mac. can you start a bug on > that? It's only on the trunk that the tab drag-scrolling is jerky (or non-existent). I've opened a new bug for it... bug 348526.