Summary: | Add an internal targetOrigin member to MessageEvent | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Karl Koscher <supersat> | ||||||||
Component: | UI Events | Assignee: | Karl Koscher <supersat> | ||||||||
Status: | RESOLVED WONTFIX | ||||||||||
Severity: | Normal | CC: | abarth, ap, creis, dglazkov, fishd, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 73337 | ||||||||||
Attachments: |
|
Description
Karl Koscher
2011-11-29 11:12:10 PST
Created attachment 117007 [details]
Patch
Comment on attachment 117007 [details] Patch Attachment 117007 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10695048 New failing tests: http/tests/inspector/extensions-headers.html http/tests/appcache/offline-access.html http/tests/appcache/main-resource-hash.html http/tests/appcache/deferred-events-delete-while-raising.html http/tests/appcache/reload.html http/tests/messaging/cross-domain-message-event-dispatch.html http/tests/appcache/cyrillic-uri.html http/tests/appcache/top-frame-4.html http/tests/appcache/xhr-foreign-resource.html http/tests/inspector/extensions-network-redirect.html http/tests/inspector-enabled/console-log-before-frame-navigation.html http/tests/inspector/extensions-ignore-cache.html http/tests/appcache/top-frame-3.html http/tests/appcache/remove-cache.html http/tests/appcache/top-frame-1.html http/tests/cache/history-only-cached-subresource-loads-max-age-https.html http/tests/inspector/extensions-useragent.html http/tests/cache/history-only-cached-subresource-loads.html http/tests/history/cross-origin-replace-history-object.html http/tests/messaging/cross-domain-message-send.html That looks like an unlikely list of failing tests. Most or all of these tests should be using postMessage, so it seems quite possible that they are genuinely broken by this patch. Created attachment 117451 [details]
Patch
Comment on attachment 117451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117451&action=review > Source/WebCore/dom/MessageEvent.cpp:87 > , m_origin("") > , m_lastEventId("") > + , m_targetOrigin("") Passing "" is unnecessarily inefficient. We should use emptyString() instead. > Source/WebCore/dom/MessageEvent.h:138 > + // For internal use only -- not exposed to any bindings I think this is a confusing comment. There are many data members in many classes with this property. A more helpful comment would explain the purpose of this, why it exists, and that might make it clear why it would not be exposed. Generally, comments should say “why” rather than “what”. Created attachment 117467 [details]
Patch
Comment on attachment 117467 [details] Patch Marking this patch as potentially obsolete. If we're not using native event handlers (see bug 73359 for a discussion of that), we can probably just pass the target origin directly. We're now going to ask the embedder if it wants to deliver the postMessage, making this bug obsolete. |