Closed
Bug 646038
Opened 14 years ago
Closed 13 years ago
URL destination flickers when the mouse pointer is over a link positioned at the bottom corner
Categories
(Firefox :: General, defect)
Tracking
()
RESOLVED
FIXED
Firefox 12
Tracking | Status | |
---|---|---|
firefox5 | - | --- |
People
(Reporter: ogirtd, Assigned: dao)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
I've added the testcase to the URL field, in order to make it easier to test.
Comment 2•14 years ago
|
||
Confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Core → Firefox
QA Contact: general → general
Whiteboard: DUPEME
Version: unspecified → 4.0 Branch
Comment 3•14 years ago
|
||
Presumably because the status thingie is shown and then moved? Can we reverse the order of those?
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•14 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:
--- → ?
Updated•14 years ago
|
Whiteboard: DUPEME
Comment 7•14 years ago
|
||
(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•14 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.
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #532196 -
Flags: review?(gavin.sharp)
Comment 11•14 years ago
|
||
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•14 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•14 years ago
|
||
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)
Comment 14•14 years ago
|
||
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•14 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 16•14 years ago
|
||
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•14 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•14 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•14 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•14 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•14 years ago
|
||
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)
Comment 27•14 years ago
|
||
> 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•14 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•14 years ago
|
||
updated to tip
Attachment #533225 -
Attachment is obsolete: true
Attachment #533225 -
Flags: review?(gavin.sharp)
Attachment #534313 -
Flags: review?(gavin.sharp)
Comment 30•13 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•13 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•13 years ago
|
||
updated to tip
Attachment #534313 -
Attachment is obsolete: true
Attachment #534313 -
Flags: review?(gavin.sharp)
Attachment #562738 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•13 years ago
|
Attachment #562738 -
Flags: review?(gavin.sharp) → review?(dolske)
Updated•13 years ago
|
Attachment #562738 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 34•13 years ago
|
||
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
Comment 35•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•