Closed
Bug 593959
Opened 14 years ago
Closed 14 years ago
:active tracking gets out of sync when releasing the mouse over a different document
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: mstange, Assigned: smaug)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Steps to reproduce:
1. Press the left mouse button on the refresh button in the browser chrome.
2. While holding the button down, move the mouse away from the refresh button, down into the content viewport.
3. Release the mouse button.
4. Move the mouse back over the refresh button.
Notice that the button now has the pressed appearance (:hover:active) instead of the merely hovered appearance.
My guess about what's happening:
nsEventStateManager probably assumes that it will get a mouseup for every mousedown that it receives. Before bug 130078 that usually happened (except when going into drag mode, bug 379272) because the widget was doing mouse capturing. Now there's no automatic mouse capturing at the chrome-content boundary anymore, so the mouseup gets sent to a different nsEventStateManager, so mActiveContent doesn't get cleared properly.
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 1•14 years ago
|
||
I'm about to fix something quite close to this in bug 524037, so I can
take a look at this too.
Assignee: nobody → Olli.Pettay
Updated•14 years ago
|
blocking2.0: ? → betaN+
Assignee | ||
Comment 2•14 years ago
|
||
This should work reasonable well.
I need to write still a mochitest for this.
Attachment #473520 -
Flags: review?(roc)
Reporter | ||
Comment 3•14 years ago
|
||
Instead of adding a global for the active ESM, couldn't you just make mActiveContent global?
Moreover, I've just discovered that there's a very similar problem with hover: It can happen that there's hovered content in one ESM and then a mouseexit event is sent into a different ESM. This event then fails to clear mHoverContent in the previously-hovered ESM, and it also fails to send the right mouseout events. (Missing mouseout events can result in sticky tooltips, for example, which I've seen frequently since bug 130078 landed.)
This condition will be easily reproducible after bug 547787, which will remove the gap between the tabs and the chrome-content edge. Then you can reproduce it like this (on Mac):
1. Open a Firefox window and deactivate it.
2. Move the mouse over a tab and wait for the tooltip to appear.
3. Move the mouse down into the content viewport.
The tab will stay hovered and the tooltip doesn't go away.
These STR result in mousemove events on the chrome ESM followed by a mouseexit event on the content ESM. This mouseexit event is sent because content in inactive windows shouldn't receive mouse events.
This didn't happen before bug 130078 because our widget code would detect that the mouse has moved into a different widget. So it would send a mouse exit event to the previously-hovered widget which would end up in the chrome ESM.
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> Instead of adding a global for the active ESM, couldn't you just make
> mActiveContent global?
That should work too, but would cause some more code changes.
sActiveESM could be used also to fix :hover.
The good thing using global ESM is that we get mPrescontext easily.
I'll update the patch.
Summary: :active tracking gets out of sync when releasing the mouse over a different document → :active/:hover tracking gets out of sync when releasing the mouse over a different document
Assignee | ||
Updated•14 years ago
|
Attachment #473520 -
Flags: review?(roc)
Comment 6•14 years ago
|
||
This also affects content, fwiw.
Assignee | ||
Comment 7•14 years ago
|
||
Markus, does this fix also the hover problem you're seeing?
(I don't have a mac nearby.)
This does fix other similar sounding hover problems.
Attachment #473520 -
Attachment is obsolete: true
Attachment #474817 -
Flags: review?(mstange)
Reporter | ||
Comment 8•14 years ago
|
||
It doesn't fix the hover problem I'm seeing. I'll try to find out why.
Reporter | ||
Comment 9•14 years ago
|
||
Ah, because the problem I'm seeing doesn't really have to do with mHoverContent, but with mLastMouseOverElement.
In nsEventStateManager::NotifyMouseOut, we only send a mouse out event (and unset :hover) if this ESM has a mLastMouseOverElement, otherwise we return early. But the right mLastMouseOverElement is in a different ESM...
Assignee | ||
Comment 10•14 years ago
|
||
Comment on attachment 474817 [details] [diff] [review]
patch
This is actually wrong also in that :hover state should stay active
in the ancestor ESM.
Attachment #474817 -
Attachment is obsolete: true
Attachment #474817 -
Flags: review?(mstange)
Assignee | ||
Comment 11•14 years ago
|
||
(In reply to comment #6)
> This also affects content, fwiw.
In which way?
Assignee | ||
Comment 12•14 years ago
|
||
So what does bug 547787 actually do?
Does it add :hover to something which is an ancestor of <browser>?
Assignee | ||
Comment 13•14 years ago
|
||
I think I'll split the :hover handling to a new bug.
Assignee | ||
Updated•14 years ago
|
Summary: :active/:hover tracking gets out of sync when releasing the mouse over a different document → :active tracking gets out of sync when releasing the mouse over a different document
Reporter | ||
Comment 14•14 years ago
|
||
(In reply to comment #12)
> So what does bug 547787 actually do?
I shouldn't have mentioned bug 547787... the problem is reproducible without it.
> Does it add :hover to something which is an ancestor of <browser>?
It uses :hover on something which is directly adjacent to the <browser>.
Before it, the tabs had a little gap under them, so when you moved your mouse over a tab and down into content, you could have a mouse out event on the tab before you entered content.
I'm talking about tabs-on-bottom, of course.
Assignee | ||
Comment 15•14 years ago
|
||
Attachment #475515 -
Flags: review?(roc)
Comment on attachment 475515 [details] [diff] [review]
patch
Test?
Attachment #475515 -
Flags: review?(roc) → review+
Assignee | ||
Comment 17•14 years ago
|
||
Will write a test before pushing this.
Assignee | ||
Comment 18•14 years ago
|
||
Assignee | ||
Comment 19•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•