Closed Bug 596678 Opened 14 years ago Closed 14 years ago

Link hover in location bar: link appears instantly, without fade, if a link is previously moused over too quickly to show it

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b7

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file, 2 obsolete files)

If you quickly mouse-over and mouse-out a link so that it does not appear in the location bar and then mouse-over that link (or any other link) again, it appears instantly in the location bar. It should instead fade in after some delay as normal. The problem is that neither the fade-in nor fade-out transitions ever actually starts, despite setting and removing the overlinkstate attribute, which means they never end, which means transitionend isn't fired, which means _overLinkTransitioning is not set to false after having been set to true. So the next time you mouse-over, _overLinkTransitioning is true, and the link is shown instantly.
Hmm, the best way I can think of to fix this is to break the transition in two: fire a timer on mouse-over that implements the initial delay, and when the timer is done set the overlinkstate attribute to start the CSS transitions. The timer can be canceled on mouse-out so that the CSS transitions never start. I was hoping to do everything with CSS though... I wish there were a transitionstart event.
Just talked to David Baron, and my understanding in comment 0 is slightly wrong. transitionend is only fired when a transition has run to completion. So the transitions may (or may not) be starting, but at least they're not ending, which is obvious. (And there're no transitioncanceled or transitionstart events.)
Attached patch patch (obsolete) (deleted) — Splinter Review
This patch does what comment 1 says: use a timeout for the initial delay. It also: * increases both the mouseover and mouseout delays to 100ms, which makes it less likely to trigger the over-link by mistake and gives you a little more time to mouse from link to link without the over-link disappearing in between. * changes the over-link to fade in/out with the default "ease" transition function, which is snappier. I shouldn't have used a custom cubic-bezier and linear function here; those were only needed on the textbox and origin URL label to make that transition look OK. This makes the over-link and origin URL fade in/out at slightly different rates, but I think it looks better overall.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Attachment #475739 - Flags: review?(dao)
Comment on attachment 475739 [details] [diff] [review] patch initialize _overLinkDelayTimer in a <field>
Attachment #475739 - Flags: review?(dao) → review+
Blocks: 596921
Attached patch patch 2 (obsolete) (deleted) — Splinter Review
Addresses comment 5. Thanks Dão.
Attachment #475739 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7
Had to back out because browser_overLinkInLocationBar.js timed out on a Linux debug Mochitest box.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I don't see much delay here, its still almost instant.
Think I have a fix. The test failure exposed a real problem in my initial patch: _overLinkTransitioning is not reliable. It's not reliable because the transitionend event is not reliable: if you start to transition the opacity to zero and therefore expect to receive transitionend, but the opacity is already zero, naturally transitionend isn't fired. The patch here tickles such a corner case. overlinkstate is set, but before its opacity actually moves off zero, overlinkstate is removed. What _overLinkTransitioning is really trying to keep track of is whether the over-link's opacity is non-zero. So get rid of it and replace it with a direct test of opacity. Will push to tryserver to see what happens, attach patch if OK.
Attached patch patch 3 (deleted) — Splinter Review
Tryserver still coming in, but so far this patch is passing where the old one failed. This does what comment 10 says. It also makes these other changes from the previous patch: * If _overLinkDelayTimer is already scheduled when setOverLink is called, it's canceled immediately (and restarted if needed). So if you drag your mouse quickly up a long list of links for example, the over-link won't appear until you stop on one. * Simplifies the test by calling setOverLink directly rather than synthesizing a mouse event on a link. I'm not testing the wiring between mouseover and setOverLink but setOverLink itself. This also avoids potential focus issues.
Attachment #475907 - Attachment is obsolete: true
Attachment #476442 - Flags: review?(dao)
Attachment #476442 - Flags: review?(dao) → review+
Blocks: 597769
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: