545049 - When dragging in the bookmarks toolbar sometimes the drop indicator does not appear.

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P2)

Description

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

STRs:
- new profile
- drag Getting started from its position towards Latest Headlines.

expected
- the drop indicator should appear before Getting started

actual:
- the drop indicator does not appear

dragging back to the start of the toolbar and then forward again makes it appear again.

i noticed this while working on bug 544047. So the drop indicator has 2 regressions, but they are not the same.

regrange:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=120667a01fd2&tochange=a43e2f7eda8f

Excluding the TM merge (for now) the most plausible bugs are bug 526394 or bug
529670.

I've checked that our code is acting correctly, but looks like the indicator even if it's in the right position is not drawn on screen.

Still investigating to reduce the range further.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

taking till i get a regressing bug.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

confirming regression from bug 526394.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

restricted between 8c1587eac4b2 (works, Part18) and 47cd92d616d7 (broken, Part36)

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

restricted more between fada8c5cef07 (works, Part30) and and 47cd92d616d7 (broken,
Part36)

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i cannot restrict more since looks like it does not compile in the middle (i did not try all 5 changesets though).

final range: between fada8c5cef07 (works, Part30) and and 9584d6bce9c1
(broken, Part34)

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Part 34 is innocent (it's already broken at Part 33).

Comment 7

[Pardal Freudenthal (:ShareBird)]

I've noticed that the indicator only appears if you are dragging from left to the right. If you drag from right to the left it doesn't appear, only if you go ahead and come back (the same as if you drag from left to the right)

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: testcase

The problem is actually a fundamental one with our implementation of CSS transforms, as far as I can tell.

When the transform changes, we need to invalidate the old and new areas covered by the transformed frame. We invalidate the old overflow area in DoApplyRenderingChangeToTree. But the new overflow area isn't calculated until we reflow the transformed frame, and when that reflow occurs, nothing really moves, and nothing triggers invalidation of the new overflow area.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think that whenever we have a transformed frame and the overflow area changes, we should invalidate the entire overflow area.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

We already have code to invalidate the entire overflow area when it changes, used for SVG effects and outline rects which already have this problem. Using it for transforms too makes sense.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Use InvalidateOverflowArea

While I was looking into this, I noticed we have some places that do aFrame->Invalidate(aFrame->GetOverflowRect()). This is obviously supposed to just invalidate the entire drawn area of the frame subtree, so we can change these to aFrame->InvalidateOverflowArea and make InvalidateOverflowArea specifically invalidate GetOverflowAreaRelativeToSelf(). This is basically just cleanup, since Invalidate is already hacked to accept coordinates either pre-transform or post-transform... But we should fix that one day.

Comment 12

[Mats Palmgren (inactive)]

Comment on attachment 434154 [details] [diff] [review]
fix

r=mats

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/f25fbbe3e978
http://hg.mozilla.org/mozilla-central/rev/b5ebe19e4ecf

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with with builds on Windows and OS X like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a4pre) Gecko/20100328 Minefield/3.7a4pre ID:20100328204715

Coved by Litmus in https://litmus.mozilla.org/show_test.cgi?id=10006

Comment 15

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

hum, i can still reproduce in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100328 Minefield/3.7a4pre
i i drag toward left i don't see the indicator... am i missing something? Henrik?

Comment 16

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

uh. it's probably in the next build, sorry!
Why the user agent has not  time indication but only date :\

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I stumbled about the same problem this morning. And yes, I should be in the todays build. The verification happened with a tinderbox build from 8:45pm which already includes this fix.

Comment 18

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

indeed this is perfectly fixed in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100329 Minefield/3.7a4pre

Thank you Robert!

Henrik, what about filing a bug to get back times in nightlies UA?

Comment 19

[Timothy Nikkel (:tnikkel)]

The reason the time is not in the user agent string is that there was a bug where the time in user agent string screwed up the user agent sniffing of some sites. I can't locate the bug now unfortunately.

Comment 20

[Dão Gottwald [:dao]]

bug 450973