Closed Bug 1167607 Opened 5 years ago Closed 5 years ago

Move AsyncEventDispatcher from using DispatchChromeEvent

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(1 file, 2 obsolete files)

Currently, AsyncEventDispatcher uses nsContentUtils::DispatchChromeEvent for chrome-only event. It could cause undesired behavior if the target is in chrome. We should make it use normal DispatchEvent method with mOnlyChromeDispatch flag set.

As noted by :smaug, we should rename {a,e}DispatchChromeOnly to {a,e}OnlyChromeDispatch, and add comment to describe the behavior.

This changes the current behavior of that class, but it seems to be fine based on test result from try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa5369bee7a2
Blocks: 1161802
Attached patch patch (obsolete) — Splinter Review
Attachment #8609905 - Flags: review?(bugs)
Blocks: 1167890
Attached patch patch (obsolete) — Splinter Review
With the renaming, found some other place to change. The new try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22327de3e9ff
Attachment #8609905 - Attachment is obsolete: true
Attachment #8609905 - Flags: review?(bugs)
Attachment #8609908 - Flags: review?(bugs)
Attached patch patchSplinter Review
Add the fix for the failure on the bc1 test. New try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=84472593c13f
Attachment #8609908 - Attachment is obsolete: true
Attachment #8609908 - Flags: review?(bugs)
Attachment #8609941 - Flags: review?(bugs)
Comment on attachment 8609941 [details] [diff] [review]
patch


>@@ -4237,17 +4237,17 @@ nsDocument::AddStyleSheetToStyleSets(nsI
>     init.memberName = argName;                                                \
>                                                                               \
>     nsRefPtr<className> event =                                               \
>       className::Constructor(this, NS_LITERAL_STRING(type), init);            \
>     event->SetTrusted(true);                                                  \
>     event->SetTarget(this);                                                   \
>     nsRefPtr<AsyncEventDispatcher> asyncDispatcher =                          \
>       new AsyncEventDispatcher(this, event);                                  \
>-    asyncDispatcher->mDispatchChromeOnly = true;                              \
>+    asyncDispatcher->mOnlyChromeDispatch = true;                              \


oh, tiny bit surprising code. I would have expected the event to have mOnlyChromeDispatch flag.
But no need to change, not about this bug.
Attachment #8609941 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/51b6ac207b1a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.