Closed Bug 484658 Opened 11 years ago Closed 11 years ago

[FIX] Fix element.focus() handing when called during event suppression

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

Details

(Keywords: fixed1.9.1)

Attachments

(2 files)

Attached patch patchSplinter Review
Currently element.focus() doesn't fire focus event if events are suppressed.
I think it should. We should just try to suppress focus/blur which are
caused by user interaction.

This may help with bug 484125.

The idea with the patch is to suppress events only if we're currently handling
NS_GOT/LOST_FOCUS or NS_DEACTIVATE/ACTIVATE and focus/blur is dispatched
to a document which should suppress events. Also, unsuppress should happen
synchronously, just make firing delayed events asynchronous.
Attachment #368786 - Flags: superreview?(jst)
Attachment #368786 - Flags: review?(jst)
Seems like the patch doesn't fix bug 484125, but it does fix the bug the
testcase is testing.
No longer blocks: 484125
Flags: blocking1.9.1?
Attachment #368786 - Flags: superreview?(jst)
Attachment #368786 - Flags: superreview+
Attachment #368786 - Flags: review?(jst)
Attachment #368786 - Flags: review+
Summary: Fix element.focus() handing when called during event suppression → [FIX] Fix element.focus() handing when called during event suppression
http://hg.mozilla.org/mozilla-central/rev/49dbc8dc6843
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
I don't think this blocks, but feel free to nominate for approval.
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Attachment #368786 - Flags: approval1.9.1?
Comment on attachment 368786 [details] [diff] [review]
patch

This should reduce risk for regression which the sync-XHR + event suppression may cause. See the testcase in the patch.
Note, if we want this for 1.9.1, this really should go to b4.
jst: this looks a little scary to me, should we take it?
I think so, because we want the sync-XHR thing.
Attachment #368786 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 368786 [details] [diff] [review]
patch

I say we want this. I agree it looks a bit scary, but I don't think it's that bad, and it's been on the trunk for some time w/o any known problems.
I'll land this tomorrow.
Attached patch 191 patchSplinter Review
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.