646038 - URL destination flickers when the mouse pointer is over a link positioned at the bottom corner

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[dror3go]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: 

When moving the mouse over a link positioned at the bottom left (bottom right in RTL builds of Firefox), the cursor flickers once. At that specific moment, the link is not clickable (clicking will not go to the destination URL).

Testcase:
data:text/html,<a href="http://example.net/" style="position:absolute; bottom: 0.5em;left:0">HOVER ME</a>

* RTL users should change left:0 to right:0

Reproducible: Always

Comment 1

[Tomer Cohen :tomer]

I've added the testcase to the URL field, in order to make it easier to test.

Comment 2

[Alex Lakatos[:AlexLakatos]]

Confirmed.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Presumably because the status thingie is shown and then moved? Can we reverse the order of those?

Comment 4

[dror3go]

Am I the only one who suffers from this on a regular basic since 4.0 (at least 50 times a day, and I'm not kidding)?

Any chance to push this to 4.0.X?

Comment 5

[Tomer Cohen :tomer]

(In reply to comment #4)
> Any chance to push this to 4.0.X?
I afraid we've missed it for Firefox 4.x, but it hope we can get it in for the next version.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I see this.

Dão, could you take a look?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> Presumably because the status thingie is shown and then moved? Can we
> reverse the order of those?

It's not as simple as that, because the mouseover is what triggers the moving.

Comment 8

[Asa Dotzler [:asa]]

not going to track this for Firefox 5. there are a number of issues with the status implementation that should probably be addressed holistically.

Comment 9

[Dão Gottwald [:dao]]

Attachment: patch

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 532196 [details] [diff] [review]
patch

Seems like MousePosTracker should only register its event listeners when it has listeners to call. Also it doesn't need to store _x/_y on the object, it can just pass them directly to callListener, right? I'm actually not sure that we need the generic MousePosTracker functionality at all - do we have other potential users?

>+    this._x = event.screenX - window.mozInnerScreenX;
>+    this._y = event.screenY - window.mozInnerScreenY;

I think you probably need to consider screenPixelsPerCSSPixel here (mozInnerScreenX is CSS pixels, I think screenX are device pixels). Unless chrome prescontexts are never zoomed?

Comment 12

[Dão Gottwald [:dao]]

(In reply to comment #11)
> Seems like MousePosTracker should only register its event listeners when it
> has listeners to call.

addListener wouldn't be able to directly call the listener then. It also seems pretty clear that the status panel would add the listener soon after startup.

> Also it doesn't need to store _x/_y on the object, it
> can just pass them directly to callListener, right?

Same as above, addListener couldn't call the listener then.

> I'm actually not sure
> that we need the generic MousePosTracker functionality at all - do we have
> other potential users?

I was thinking of using it for full screen mode instead of the 1px tall black bar.

> I think you probably need to consider screenPixelsPerCSSPixel here
> (mozInnerScreenX is CSS pixels, I think screenX are device pixels). Unless
> chrome prescontexts are never zoomed?

Not sure, but it seems strange that mozInnerScreenX would be CSS pixels.

Comment 13

[Dão Gottwald [:dao]]

Attachment: patch v2

Converted event.screenX/Y to CSS pixels. Tested manually by executing this in the error console:

top.opener.getInterface(Components.interfaces.nsIWebNavigation).QueryInterface(Components.interfaces.nsIDocShell).contentViewer.QueryInterface(Components.interfaces.nsIMarkupDocumentViewer).fullZoom = 2

Comment 14

[:Gavin Sharp [email: [e-mail]]]

I still don't like having a global mousemove handler. Ideally we could poll the mouse position when we open the panel instead, but I suppose there's no easy way to do that...

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 532897 [details] [diff] [review]
patch v2

bz, do you happen to have an opinion on this? Does this look like something front-end code should avoid doing? Would an API for querying the current mouse pointer position make sense / be more efficient?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 532897 [details] [diff] [review]
patch v2

I'm going to punt to roc on this; he's thought more about those issues.

Comment 17

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

Comment on attachment 532897 [details] [diff] [review]
patch v2

Review of attachment 532897 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +8705,5 @@
> +  handleEvent: function (event) {
> +    var screenPixelsPerCSSPixel = this._windowUtils.screenPixelsPerCSSPixel;
> +    this._x = event.screenX / screenPixelsPerCSSPixel - window.mozInnerScreenX;
> +    this._y = event.screenY / screenPixelsPerCSSPixel - window.mozInnerScreenY;
> +

You shouldn't need screenPixelsPerCSSPixel here; screenX/Y should be in CSS pixels already.

@@ +8720,5 @@
> +    let rect = listener.getMouseTargetRect();
> +    let hover = this._x >= rect.left &&
> +                this._x <= rect.right &&
> +                this._y >= rect.top &&
> +                this._y <= rect.bottom;

you probably should be testing < instead of <=

Comment 18

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

So the idea here is to detect events being fired at an element that is transparent to events?

How would an API for querying the current mouse position help?

What issues am I supposed to have thought about, exactly? :-)

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #18)
> So the idea here is to detect events being fired at an element that is
> transparent to events?

Yes... I want it to respond to mouse movement but not to clicks.

> How would an API for querying the current mouse position help?
> 
> What issues am I supposed to have thought about, exactly? :-)

One problem is that when the mouse is at the lower left, the status panel should appear on the right, rather than appearing on the left and moving to the right when the mouse budges. So in order to tell where the status panel should show up initially, the current patch tracks the mouse movement in the window all the time. Should we be concerned about the overhead of this?

Comment 20

[Dão Gottwald [:dao]]

(In reply to comment #17)
> > +  handleEvent: function (event) {
> > +    var screenPixelsPerCSSPixel = this._windowUtils.screenPixelsPerCSSPixel;
> > +    this._x = event.screenX / screenPixelsPerCSSPixel - window.mozInnerScreenX;
> > +    this._y = event.screenY / screenPixelsPerCSSPixel - window.mozInnerScreenY;
> > +
> 
> You shouldn't need screenPixelsPerCSSPixel here; screenX/Y should be in CSS
> pixels already.

It appeared to deliver bogus numbers with the test from comment 13, though.

Comment 21

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

(In reply to comment #19)
> One problem is that when the mouse is at the lower left, the status panel
> should appear on the right, rather than appearing on the left and moving to
> the right when the mouse budges. So in order to tell where the status panel
> should show up initially, the current patch tracks the mouse movement in the
> window all the time. Should we be concerned about the overhead of this?

I honestly don't know. You may be forcing a reflow flush on every mouse move which we didn't need before, which could be bad on some pages.

Comment 22

[Dão Gottwald [:dao]]

So just listening to the mousemove event wouldn't be a problem, but accessing mozInnerScreenX could be?

Comment 23

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

No, it's the call to getBoundingClientRect that will cause a layout flush.

Comment 24

[Dão Gottwald [:dao]]

That's good to know as well, but when the status panel isn't visible, that call wouldn't happen -- we'd only listen to the event and store x and y.

Comment 25

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

Ah, hmm. Then this is probably OK I guess.

Comment 26

[Dão Gottwald [:dao]]

Attachment: patch v3

This avoids calling getBoundingClientRect for every mousemove event while the status panel is open, and fixes bug 629925.

Comment 27

[Boris Zbarsky [:bzbarsky]]

> What issues am I supposed to have thought about, exactly? :-)

Things like "how do we expose coordinate transformations to script" and "what should hit testing work like" and so forth... ;)

Comment 28

[Dão Gottwald [:dao]]

There's no reason why this should to be tracked for Firefox 6, although I'd certainly like to see it fixed in Firefox 6.

Comment 29

[Dão Gottwald [:dao]]

Attachment: patch v3

updated to tip

Comment 30

[Will Elwood (:Will)]

For what it's worth, my experience of this problem has been that the tooltip doesn't change sides and flickers constantly rather than once, consuming a lot of CPU until the mouse is moved away from the link. I just tried the test case in Firefox 4.0.1 and 7beta1 (win7x64 aero) to confirm the behaviour described.

Comment 31

[bugmenot]

Regarding comment 30, I noticed the same symptoms.  For me they were caused by the Redirect Remover extension.  It seems to prevent the status box from moving away from the cursor.

Comment 32

[Dão Gottwald [:dao]]

Attachment: patch v3

updated to tip

Comment 34

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/35ca03af1b5e

Comment 35

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/35ca03af1b5e