The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 12

Status

()

Firefox
General
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: dror3go, Assigned: dao)

Tracking

(Blocks: 1 bug)

4.0 Branch
Firefox 12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox5-)

Details

(URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
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

6 years ago
I've added the testcase to the URL field, in order to make it easier to test.
Confirmed.
Status: UNCONFIRMED → NEW
Component: General → General
Ever confirmed: true
Product: Core → Firefox
QA Contact: general → general
Whiteboard: DUPEME
Version: unspecified → 4.0 Branch
Presumably because the status thingie is shown and then moved? Can we reverse the order of those?
(Reporter)

Comment 4

6 years ago
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

6 years ago
(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.
tracking-firefox5: --- → ?
tracking-firefox6: --- → ?
I see this.

Dão, could you take a look?
Assignee: nobody → dao
Whiteboard: DUPEME
(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

6 years ago
not going to track this for Firefox 5. there are a number of issues with the status implementation that should probably be addressed holistically.
tracking-firefox5: ? → -
(Assignee)

Comment 9

6 years ago
Created attachment 532196 [details] [diff] [review]
patch
Attachment #532196 - Flags: review?(gavin.sharp)
(Assignee)

Updated

6 years ago
Duplicate of this bug: 634433
(Assignee)

Updated

6 years ago
Blocks: 629925
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?
Attachment #532196 - Flags: review?(gavin.sharp) → review-
(Assignee)

Comment 12

6 years ago
(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.
(Assignee)

Comment 13

6 years ago
Created attachment 532897 [details] [diff] [review]
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
Attachment #532196 - Attachment is obsolete: true
Attachment #532897 - Flags: review?(gavin.sharp)
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...
(Assignee)

Comment 15

6 years ago
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?
Attachment #532897 - Flags: feedback?(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.
Attachment #532897 - Flags: feedback?(bzbarsky) → feedback?(roc)
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 <=
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? :-)
(Assignee)

Comment 19

6 years ago
(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?
(Assignee)

Comment 20

6 years ago
(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.
(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.
(Assignee)

Comment 22

6 years ago
So just listening to the mousemove event wouldn't be a problem, but accessing mozInnerScreenX could be?
No, it's the call to getBoundingClientRect that will cause a layout flush.
(Assignee)

Comment 24

6 years ago
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.
Ah, hmm. Then this is probably OK I guess.
(Assignee)

Comment 26

6 years ago
Created attachment 533225 [details] [diff] [review]
patch v3

This avoids calling getBoundingClientRect for every mousemove event while the status panel is open, and fixes bug 629925.
Attachment #532897 - Attachment is obsolete: true
Attachment #532897 - Flags: review?(gavin.sharp)
Attachment #532897 - Flags: feedback?(roc)
Attachment #533225 - Flags: review?(gavin.sharp)
> 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... ;)
(Assignee)

Comment 28

6 years ago
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.
tracking-firefox6: ? → ---
(Assignee)

Comment 29

6 years ago
Created attachment 534313 [details] [diff] [review]
patch v3

updated to tip
Attachment #533225 - Attachment is obsolete: true
Attachment #533225 - Flags: review?(gavin.sharp)
Attachment #534313 - Flags: review?(gavin.sharp)

Comment 30

6 years ago
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

6 years ago
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.
(Assignee)

Comment 32

6 years ago
Created attachment 562738 [details] [diff] [review]
patch v3

updated to tip
Attachment #534313 - Attachment is obsolete: true
Attachment #534313 - Flags: review?(gavin.sharp)
Attachment #562738 - Flags: review?(gavin.sharp)
(Assignee)

Updated

5 years ago
Duplicate of this bug: 713808
(Assignee)

Updated

5 years ago
Attachment #562738 - Flags: review?(gavin.sharp) → review?(dolske)
Attachment #562738 - Flags: review?(dolske) → review+
(Assignee)

Comment 34

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/35ca03af1b5e
Summary: URL destination flickers once when mouse over (hover) a link positioned at the bottom corner → URL destination flickers when the mouse pointer is over a link positioned at the bottom corner
Target Milestone: --- → Firefox 12
(Assignee)

Updated

5 years ago
Blocks: 717752
https://hg.mozilla.org/mozilla-central/rev/35ca03af1b5e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 727793

Updated

5 years ago
Depends on: 733284

Updated

5 years ago
No longer blocks: 717752

Updated

5 years ago
Blocks: 717752
You need to log in before you can comment on or make changes to this bug.