[webkit-reviews] review granted: [Bug 17331] Change postMessage/MessageEvent to match HTML5 wrt. exposing origin vs. domain/uri : [Attachment 20631] Updated patch to address aroben's comments

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 18 10:05:57 PDT 2008


Adam Roben (aroben) <aroben at apple.com> has granted Collin Jackson
<collinj-webkit at collinjackson.com>'s request for review:
Bug 17331: Change postMessage/MessageEvent to match HTML5 wrt. exposing origin
vs. domain/uri
http://bugs.webkit.org/show_bug.cgi?id=17331

Attachment 20631: Updated patch to address aroben's comments
http://bugs.webkit.org/attachment.cgi?id=20631&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
r31975 added protocolHostAndPortAreEqual to KURL.h. Should we use that instead
of adding SecurityOrigin::isSameSchemeHostPort?

+    // Sender is not allowed to see exceptions other than syntax errors
+    ExceptionCode ec; 
+    document()->dispatchEvent(new MessageEvent(message, sourceOrigin, source),
ec, true);

Should ec be initialized to 0?

+	 [DoNotCheckDomainSecurity, Custom] void postMessage(in DOMString
message, in [Optional] DOMString origin)

Let's call it targetOrigin here as well.

+    if (m_port) {
+      append(result, ":");
+      append(result, String::number(m_port));
+    }

Looks like the indentation got messed up here.

+	 // Okay deleted value because "invalid-protocol" is not a valid
protocol.

What makes it invalid?

You should use svn cp when renaming the tests so that the history is preserved
(maybe	you already did this and it didn't come through in the diff).

r=me, but feel free to fix the above. You may also want to get Sam Weinig to
give this a quick once-over before landing.


More information about the webkit-reviews mailing list