Bug 218546 - window.event may get set on wrong global when dispatching an event
Summary: window.event may get set on wrong global when dispatching an event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://dom.spec.whatwg.org/#concept-...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-03 16:40 PST by Chris Dumez
Modified: 2020-11-04 18:24 PST (History)
9 users (show)

See Also:


Attachments
WIP Patch (3.24 KB, patch)
2020-11-03 16:41 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.47 KB, patch)
2020-11-04 07:54 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.62 KB, patch)
2020-11-04 08:05 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (18.15 KB, patch)
2020-11-04 11:05 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (105.55 KB, patch)
2020-11-04 11:10 PST, Chris Dumez
sam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (18.13 KB, patch)
2020-11-04 16:11 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-11-03 16:40:39 PST
window.event may get set on wrong global when dispatching an event.
Comment 1 Chris Dumez 2020-11-03 16:41:03 PST
Created attachment 413122 [details]
WIP Patch
Comment 2 Chris Dumez 2020-11-04 07:54:17 PST
Created attachment 413163 [details]
Patch
Comment 3 Chris Dumez 2020-11-04 08:05:09 PST
Created attachment 413165 [details]
Patch
Comment 4 Geoffrey Garen 2020-11-04 10:31:15 PST
Comment on attachment 413165 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413165&action=review

> Source/WebCore/ChangeLog:9
> +        'current event', stating that global the 'listener callbackâs associated Realmâs global

that global the => that global is the

> Source/WebCore/bindings/js/JSDOMWindowBase.h:114
> +    Event* m_currentEvent { nullptr };

Can we use RefPtr here?

> Source/WebCore/bindings/js/JSErrorHandler.cpp:83
> +        Event* savedEvent = nullptr;

Can we use RefPtr here?

> Source/WebCore/bindings/js/JSErrorHandler.cpp:86
> +            jsFunctionWindow->currentEvent();

I think you meant to assign to savedEvent here? Seems like a bug. Might need a new test.

> Source/WebCore/bindings/js/JSEventListener.cpp:166
> +    Event* savedEvent = nullptr;

Can we use RefPtr here?
Comment 5 Chris Dumez 2020-11-04 10:54:00 PST
Comment on attachment 413165 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413165&action=review

>> Source/WebCore/bindings/js/JSDOMWindowBase.h:114
>> +    Event* m_currentEvent { nullptr };
> 
> Can we use RefPtr here?

I am merely moving the code so this is not new. I think we can make it a RefPtr though, even if not strictly needed for safety.
Comment 6 Chris Dumez 2020-11-04 11:05:22 PST
Created attachment 413182 [details]
Patch
Comment 7 Chris Dumez 2020-11-04 11:10:54 PST
Created attachment 413184 [details]
Patch
Comment 8 Chris Dumez 2020-11-04 16:11:35 PST
Created attachment 413221 [details]
Patch
Comment 9 EWS 2020-11-04 18:23:36 PST
Committed r269414: <https://trac.webkit.org/changeset/269414>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413221 [details].
Comment 10 Radar WebKit Bug Importer 2020-11-04 18:24:21 PST
<rdar://problem/71060677>