306149 - hover dynamic after Rightclick/saveAs

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050826 SeaMonkey/1.1a

If seen this bug for some time on some pages.  Normally, when I hover over a
link, the title stays in the status bar until the mouse leaves the link.
After I've done a download using right-click/saveAs, hover is seen only while
the mouse is moving. Also Focus changes between Link and Reload button.

Steps to repeat on testcase:
1. Load testcase
2. Hover over 'M' to check normal behavior
3. Rightclick 'M' and Save As.
4. Hover over 'M' to see the bug:
title is seen shortly only in statusbar, Reload Button frequently gets Focus
while mouse is hovering.

Comment 1

[Hermann Schwab]

Attachment: reduced testcase

Download with rightclick/SaveAs on the 'M', and then look at hover.
Changing to another tab or hovering over chrome fixes it.

Comment 2

[Hermann Schwab]

you don't have to save the file, the bug is also seen if you cancel the file
selector box.
Bug can be seen in the wild here:
http://www.atmel.com/dyn/products/datasheets.asp?family_id=605
right-click on one of the pdf symbols,´Save As and cancel on the file selector box.

It's a bit more complicated to see it at ST:
http://www.st.com/stonline/books/toc/an/1138.htm
Here you must scroll a line or two before trying the download.
Bug is not seen when page is not scrolled.
As this bug is seen in tables, maybe it is a Layout/Table bug?

Comment 3

[Martijn Wargers (dead)]

This bug has the same symptons as what bug 290673 was about.

Comment 4

[Martijn Wargers (dead)]

Attachment: Testcase2

Comment 5

[Martijn Wargers (dead)]

This regressed betweeen 2005-03-28 and 2005-03-29. The only fix, likely to have
caused this, is the fix bug 284664, I think.

Comment 6

[Martijn Wargers (dead)]

Happens also when right-clicking any link and then choosing any menu-item in the
popup and then hovering over the link again.

Comment 7

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

I only just noticed that this is windows only :-(

Comment 8

[Bruno 'Aqualon' Escherl]

WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051006
SeaMonkey/1.0b

Both testcases work as expected and don't show the described wrong behaviour.

Comment 9

[Bruno 'Aqualon' Escherl]

Regarding bug 294060, perhaps it's only a problem with Win9x?

Comment 10

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051012 SeaMonkey/1.1a

(In reply to comment #9)
> Regarding bug 294060, perhaps it's only a problem with Win9x?

I'm still seeing the bug on reduced testcase attachment 194015 [details], and if I exactly
follow the steps mentioned in Attachment 194060 [details] I also can see the bug (Didn't
see it at first try)

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I've seen states like this sometimes but never been able to repro -- I've suspect something related to the changes for bug 20022.

Comment 12

[Benoît]

I just tested this under Firefox 2.0 Final. It's there too.

Comment 13

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

Hum. Is this really Win9x only? If it is, I think it may not be worth fixing.

Comment 14

[Benoît]

No, I see it on Windows XP as well.

Comment 15

[Ere Maijala (slow)]

This happens because after displaying the popup menu every mouse move over content window causes a chrome reflow which uses wrong mouse coordinates (chrome view manager as opposed to content view manager) to synthesize a mouse move. I don't know yet why the reflows happen.

Comment 16

[Benoît]

That's very helpful, Ere Maijala. It's possible you'll find more information in the bugs that this one blocks. 284664 was the original one, which was partly fixed, and led to 294060. The all have similar symptoms to this one.

Comment 17

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

Attachment: fix?

Try this!

Comment 18

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

Comment on attachment 245708 [details] [diff] [review]
fix?

Ere says this works. The approach is straightforward: only remember mouse locations and synthesize mouse events in the root view manager. This way we don't have to worry about view managers have inconsistent saved mouse locations.

Comment 19

[Benoît]

I'd like to test this on branch, but the patch doesn't work on it. Can someone advice how I get it in, or is a different version needed?

Comment 20

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

The patch should work on branch. It may require manual merging.

Comment 21

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

Checked in

Comment 22

[Benoît]

Attachment: branch version of the fix

I hope it's alright that I attach the branch version that I made myself to this bug.

Comment 23

[Ria Klaassen (not reading all bugmail)]

*** Bug 360714 has been marked as a duplicate of this bug. ***

Comment 24

[Benoît]

Sadly, this fix does NOT work on branch.

I'm waiting for a tinderbox build from after the check-in.

Comment 25

[Benoît]

I confirm that the issue is fixed on the trunk.
I hope I don't have to open yet another bug for the branch...

Comment 26

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

It may not be worth fixing on the branch.

Comment 27

[Benoît]

A regression that hasn't been fixed since *MAY 2005* isn't worth fixing?!
I didn't offer my help to be told that I didn't matter.

This fix can still get in SeaMonkey 1.1, so it's definitely worth fixing.
I'm not the only one who gets annoyed at this bug every day!

Comment 28

[Benoît]

Now my compiled branch build doesn't show the bug anymore. This is really strange.

I'm going to let others on MozillaZine that experienced the bug to test my build.

Comment 29

[Martijn Wargers (dead)]

Comment on attachment 245708 [details] [diff] [review]
fix?

(In reply to comment #28)
> Now my compiled branch build doesn't show the bug anymore. This is really
> strange.

Perhaps you didn't rebuild enough after you applied the patch? Just rebuilding view/ isn't enough.
This bug is annoying, I agree, so I'm going to ask approval 1.8.1.1 for it. I know some people hit this bug a lot of times. But don't count on it that it does get approved.

Comment 30

[Martijn Wargers (dead)]

Comment on attachment 245708 [details] [diff] [review]
fix?

I'm pulling out my nomination of the patch, since it already caused one known regression. If you want this to be on the 1.8.1.1 release, you'll going to have to nominate it yourself.

Comment 31

[Benoît]

Which regression? I don't think that would be bug 337915, because the patch hasn't been checked in on branch yet.

I noticed that the branch version of the patch now requires you to click twice on a link, though. Maybe that's it.

> Perhaps you didn't rebuild enough after you applied the patch?

I deleted the entire objdir before compiling, so no, that's not the case.

Comment 32

[Krang]

Comment on attachment 245772 [details] [diff] [review]
branch version of the fix

>+            RootViewManager()->mMouseLocation = aEvent->refPoint +

Could you try changing aEvent->refPoint to aEvent->point here to see if it makes the branch patch work?

Comment 33

[Benoît]

Other regressions that the branch version of the patch introduces:

In context menus, especially those in Mail & Newsgroups, you get the crazy mouse bug often. Especially in the submenus.

While the page is loading, you always get the crazy mouse bug when you hover over a link. The bug disappears as soon as the page has finished loading.

As for clicking the link twice, I've noticed that the first time you click the mouse pointer briefly becomes a "forbidden" icon, as if it thought you were trying to drag it.

ParadigmK, I'll try that, thanks.

Comment 34

[Benoît]

Okay, I compiled a branch build with the change, but it didn't make the patch work. It even unfixed this bug, I can still reproduce it.

Comment 35

[Krang]

(In reply to comment #34)
> Okay, I compiled a branch build with the change, but it didn't make the patch
> work. It even unfixed this bug, I can still reproduce it.

Sorry about that.  I'm not sure what else could be wrong with the branch patch, so we need someone who knows the APIs better to take a look at this.  Did it fix any of the branch regressions at least?

Comment 36

[Benoît]

It seems like it, I didn't see the branch-only regressions.

Comment 37

[Martijn Wargers (dead)]

(In reply to comment #31)
> Which regression? I don't think that would be bug 337915, because the patch
> hasn't been checked in on branch yet.

Bug 361042, it is blocking this bug.

Comment 38

[Boris Zbarsky [:bzbarsky]]

On branch, point and refPoint mean slightly different things, both different from what what refPoint means on trunk.  Bug 296036 comes to mind, for example.

I pretty much agree with roc; trying to deal with this on branch is likely to be a painful waste of everyone's time.

Comment 39

[Benoît]

I would advise we find out why the chrome reflow happens instead of coming up with workarounds.

Comment 40

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

We know why the chrome reflow happens. It happens because the status bar changes. That is correct. The trunk fix is the correct fix, not a workaround.

Comment 41

[Benoît]

Okay, but then chrome reflows would be normal. What caused the crazy mouse then? If it's wrong coordinates, what were those caused by?

Comment 42

[Boris Zbarsky [:bzbarsky]]

Did you read comment 38?  Mouse coordinates on trunk work pretty differently from mouse coordinates on branch in many cases.

Comment 43

[Benoît]

Yes, I read it, and it is irrelevant to my question.

I was asking why this bug happened, why it got wrong coordinates, since the chrome reflow seems to be normal.

This bug happened on BOTH branch and trunk.

Comment 44

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

The problem is that mouse events were being synthesized by the chrome view manager which hadn't been updated to the correct mouse location (those updates were performed by the content-pane view manager). The patch prevents this by forcing all event synthesis and mouse location remembering to be done by the root (chrome) view manager.

Some version of this patch would work on branch. It's just not completely trivial to do the backport. If this bug is serious enough we could do that backport but I'm not convinced it is, or that it would get branch approval if we did it.

Comment 45

[Ria Klaassen (not reading all bugmail)]

*** Bug 362097 has been marked as a duplicate of this bug. ***

Comment 46

[Martijn Wargers (dead)]

For people who might have interest in this, I found a way to still trigger this type of bug with almost the same steps to reproduce. I filed bug 363777 for it.