[webkit-reviews] review denied: [Bug 132269] Document.contentType implementation : [Attachment 232245] Patch v3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 29 06:37:56 PDT 2014


Csaba Osztrogonác <ossy at webkit.org> has denied Tibor Mészáros
<tmeszaros.u-szeged at partner.samsung.com>'s request for review:
Bug 132269: Document.contentType implementation
https://bugs.webkit.org/show_bug.cgi?id=132269

Attachment 232245: Patch v3
https://bugs.webkit.org/attachment.cgi?id=232245&action=review

------- Additional Comments from Csaba Osztrogonác <ossy at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=232245&action=review


> LayoutTests/fast/dom/document-contentType-data-uri.html:10
> +<iframe data-mt="text/xml"></iframe>
> +<iframe data-mt="application/xml"></iframe>
> +<iframe data-mt="image/svg+xml"></iframe>
> +<iframe data-mt="text/html"></iframe>

Changing the order of iframes isn't the proper fix here.

If text/html is the first iframe, its event still fires
at the end. It seems it isn't guaranteed that the events
fire in order. I don't know if it is normal or a bug.

Assuming that it isn't a bug, we should order the PASS messages once 
we caught all of the events in order to have deterministic test.

> LayoutTests/fast/dom/document-contentType-data-uri.html:27
> +var eventMethod = window.addEventListener ? "addEventListener" :
"attachEvent";
> +var eventer = window[eventMethod];
> +var messageEvent = eventMethod == "attachEvent" ? "onmessage" : "message";
> +
> +eventer(messageEvent,function(e) {
> +    if (e.data) {
> +	   shouldBe('"' + e.data.obtained + '"', '"' + e.data.expected + '"');
> +    } else
> +	   testFailed("Null message payload");
> +
> +    if (--expectedMessagesCount == 0)
> +	   finishJSTest();
> +},false);

What was the reason to refactor this part of the original patch?


More information about the webkit-reviews mailing list