Summary: | window.postMessage() / MessagePort.postMessage() throw wrong exception for invalid ports argument | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, haraken, japhet, kenneth, s.choi, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
URL: | http://www.w3.org/TR/html5/comms.html | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 94310 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2012-08-21 03:14:54 PDT
Created attachment 159652 [details]
Patch
Created attachment 159655 [details]
Patch
Created attachment 159656 [details]
Patch
Comment on attachment 159656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159656&action=review The change looks OK. I just want to confirm cross-browser compatibility before r+ing it. > Source/WebCore/bindings/v8/V8Utilities.cpp:117 > + if (ports.contains(port)) { > + setDOMException(INVALID_STATE_ERR, isolate); > + return false; > + } This change is trying to throw exception for the code that had not thrown exception before. What are the behaviors of other browsers (IE, Firefox and Opera)? Firefox does not seem to support MessageChannel / MessagePort at all: https://bugzilla.mozilla.org/show_bug.cgi?id=677638 https://bugzilla.mozilla.org/show_bug.cgi?id=741618 (In reply to comment #5) > Firefox does not seem to support MessageChannel / MessagePort at all: > https://bugzilla.mozilla.org/show_bug.cgi?id=677638 > https://bugzilla.mozilla.org/show_bug.cgi?id=741618 Thanks, how about Opera and IE? On Opera, it throws a DATA_CLONE_ERR instead of an INVALID_STATE_ERR, but at least it throws: PASS channel.port1.postMessage("duplicate port", [channel3.port1, channel3.port1]) threw exception DOMException: DATA_CLONE_ERR. I'm not sure how I can check for IE. Comment on attachment 159656 [details]
Patch
I checked it on IE. IE doesn't implement MessageChannel.
- IE and Firefox don't implement MessageChannel.
- Given that Opera throws exceptions, it makes sense for WebKit to throw some exception.
- Although Opera throws DATA_CLONE_ERR, the spec requires to throw INVALID_STATE_ERR. Since people wouldn't much care about the exception type, it sounds reasonable to change WebKit to throw INVALID_STATE_ERR according to the spec.
Let me r+ for now. But let's wait for landing the patch for one day, as someone might have a concern.
Comment on attachment 159656 [details]
Patch
No one has raised any concerned and the EWS bots are all green. Setting cq flag. Could someone please commit?
Comment on attachment 159656 [details]
Patch
OK, thanks for the patch.
Comment on attachment 159656 [details] Patch Clearing flags on attachment: 159656 Committed r126286: <http://trac.webkit.org/changeset/126286> All reviewed patches have been landed. Closing bug. |