280041 - :hover effect not removed completely

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Doug Wright]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050125 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050125 Firefox/1.0+

Testcase and screenshot coming up.

Table with a css :hover rule to highlight table rows as the mouse passes over
them. If I scroll whilst hovering (mousewheel), then some rows that were
previously passed over contain partially coloured rows. Re-hovering over the
rows statically (not scrolling) restore the proper colour.

Regression from 1.7.5

Reproducible: Always

Steps to Reproduce:
1. View testcase
2. Scroll whilst hovering over table
3. Watch as parts of table rows are 1 colour, other parts of the same row get a
different background.

Comment 1

[Doug Wright]

Attachment: screenshot showing problem

Comment 2

[Doug Wright]

Attachment: testcase

Comment 3

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050127
it is easier seen on Firefox trunk but also reproducable on mozilla Trunk.
I´ve used cursor keys or mousewheel to scroll while hovering, both was showing
the bug.

Comment 4

[Martijn Wargers (dead)]

I haven't seen this bug in a 2005-01-19 trunk build, but I can see it in a
2005-01-20 build.
I think this could maybe be a regression from fixing bug 244366.
Just scrolling with the mousewheel is enough for me to trigger the bug, although
I have to repeat it a few times.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hmm... I just tried the testcase in a 2005-01-25-06 Linux trunk build on Linux
and I can't reproduce (scrolling with arrow keys, since I don't have a
mousewheel).  Same for a debug Firefox build from the 25th.

So first of all, is anyone seeing this on a platform other than Windows?

Comment 6

[Martijn Wargers (dead)]

Maybe you can reproduce it when you have smooth scrolling enabled? (I could not
reproduce it with the arrow keys without smooth scrolling)

Comment 7

[Hermann Schwab]

(In reply to comment #6)
> Maybe you can reproduce it when you have smooth scrolling enabled? (I could not
> reproduce it with the arrow keys without smooth scrolling)

I don´t use smooth scrolling, as I´m on a slow celeron 333.
I crashed on 2nd testcase by just loading it and going back. (used view-source
inbetween )

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

interesting... without smooth scrolling, I can't reproduce this on
linux/gtk2/trunk (checkout finish: Mo Jan 24 19:23:08 CET 2005), but with smooth
scrolling I see it _sometimes_ (rarely). I only tried scrolling with mouse wheel
in these experiments.

Retesting using arrow keys and page up/down shows that this also works to
reproduce this bug, when smooth scroll is enabled.

since I needed to enable smooth scroll to enable this, maybe I'm seeing a
different bug. on the other hand, maybe smooth scrolling only increases the
chances of seeing this bug.

Comment 9

[Doug Wright]

The initial report was performed with smooth scrolling enabled. I've retested
with smooth scrolling disabled, and can still reproduce - but (a)not as often
and (b)it's a whole row that is left painted wrongly, and not just little bits
of several rows

Comment 10

[Boris Zbarsky [:bzbarsky]]

So how does one go about enabling smooth scrolling in a Seamonkey build?

Comment 11

[Doug Wright]

(In reply to comment #10)
> So how does one go about enabling smooth scrolling in a Seamonkey build?

It's on the very 1st Appearence tab under preferences IIRC.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Maybe this would help

I still can't reproduce this (keyboard scrolling, smooth scroll on and off,
debug firefox or opt seamonkey build), but could someone who sees it try this
patch?	It might help.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

notes:
- I'm also seeing this on checkout finish: Fr Jan 28 21:59:05 CET 2005 (2.5
hours ago)
- bz's patch does not seem to help for me

Comment 14

[Boris Zbarsky [:bzbarsky]]

OK, I finally managed to reproduce this (had to find a debug GTK2 build to do so).

As far as I can tell, it looks like the part of the patch to bug 244366 that
"caused" this bug was removing the synchronous repaint in
nsEventStateManager::PreHandleEvent.  If I put this back in, things seem to get
better.

Also, if I force the "no scrollwidget" codepath in nsScrollPortView::Scroll, the
bug seems to disappear.

I'm guessing that the sequence of events here is the following:

1)  The invalidates triggered by the style change are stored on the root view of
    the viewmanager in absolute coordinates.
2)  When we bit-blit, we move the widget without invalidating it.
3)  This ends up causing a style change, which stashes the area that stopped
    being hovered in the dirty region from step 1, and posts an event to
    invalidate later.
4)  We scroll again, bit-blit again.
5)  The invalidate is processed and invalidates the wrong part of the widget
    (since the right part has moved from the area that's marked as needing
    invalidation).

Conclusion:  It looks like the widget bit-blit needs to process pending updates,
if there are any, before blitting.

Robert, thoughts?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 16

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

What makes you think that we won't scroll while refresh is disabled?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Nothing, unfortunately.  We were already failing to handle this case, though...

The problem is that at the moment we accumulate damage areas for all sorts of
updates on the root view.  Then when we invalidate we apply the damage to all
the widgets.  If one widget moves, we need to remember to apply the damage to
that one widget in a different coordinate system from other widgets.

I guess what I could do is that if a widget moves while refresh is disabled, the
scrollable view accumulates all damage areas for all ancestors into its damage
region and offsets it accordingly.  This will cause us to do extra invalidates
for scrolling while refresh is disabled, but it's the only way I see of getting
this right given the current setup.

Comment 18

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

Yeah, that sounds OK.

Comment 19

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

Comment on attachment 172817 [details] [diff] [review]
Proposed patch

minusing for new patch

Comment 20

[Boris Zbarsky [:bzbarsky]]

Another option is that when we scroll we walk through all views, and any dirty
region that intersects the bounds of the scrollview gets copied, shifted by the
scroll amount, and Or'ed with itself.  I think that would also fix the
correctness issue here.  It'd make us do some extra painting, maybe, but maybe
not...

Comment 21

[Boris Zbarsky [:bzbarsky]]

So the difference between comment 20 and comment 17 is that we'd only shift the
part of the region that overlapped the view bounds.  Which we'd have to put in
the same coordinate system as the region and all...  Or is that adding in
unnecessary complication?

Comment 22

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch per comment 20

Note that the sign of the point we offset the intersection by is the opposite
of the nsScrollPortView.cpp code used to do.  I'm convinced that that change is
correct for two reasons:

1)  The theoretical justification in the comment when the sign is flipped
2)  The fact that without that change the bug didn't go away.  ;)

Comment 23

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 24

[Steve England [:stevee]]

This may have fixed bug 201198

Comment 25

[Doug Wright]

Confirming that today's Firefox nightly doesn't display the bug anymore.

As an aside, it's actually improved over 1.7.5 - 1.7.5 disabled :hover effects
whilst scroling, and only painted when scrolling ended - now the :hover paints
during the scrolling as well.

Thanks bz!

Comment 26

[Jacek Piskozub]

This caused build bustage on VS6, which still is not fixed at least for Windows
ME. See bug 281158.