XMLHttpRequest's responseXML should be annotated with [Exposed=Window]
Created attachment 322309 [details] Patch
Comment on attachment 322309 [details] Patch Is this not testable?
(In reply to Alexey Proskuryakov from comment #2) > Comment on attachment 322309 [details] > Patch > > Is this not testable? Yes. I was hoping there was already a test and it would fail (hence the no r?) but it looks like I’ll have to write one.
Created attachment 322317 [details] Patch
(In reply to Sam Weinig from comment #3) > (In reply to Alexey Proskuryakov from comment #2) > > Comment on attachment 322309 [details] > > Patch > > > > Is this not testable? > > Yes. By which of course I mean, “yes, it is testable” (double negative caught me). Last WIP adds the test. I’m surprised that not having a result didn’t cause the EWS to fail.
(In reply to Sam Weinig from comment #5) > (In reply to Sam Weinig from comment #3) > > (In reply to Alexey Proskuryakov from comment #2) > > > Comment on attachment 322309 [details] > > > Patch > > > > > > Is this not testable? > > > > Yes. > > By which of course I mean, “yes, it is testable” (double negative caught > me). Ahhhh. I meant negative not double negative!
Created attachment 322319 [details] Patch
Filed https://github.com/w3c/web-platform-tests/pull/7530 to add the new tests to WPT.
Comment on attachment 322319 [details] Patch Attachment 322319 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4715745 New failing tests: fast/xmlhttprequest/xmlhttprequest-responseXML-in-worker.html
Created attachment 322320 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 322321 [details] Patch
(In reply to Sam Weinig from comment #11) > Created attachment 322321 [details] > Patch New patch includes the now landed WPTs, imported back into WebKit.
> I’m surprised that not having a result didn’t cause the EWS to fail. Please feel encouraged to file a bug! I think that's a pretty bad issue.
(In reply to Alexey Proskuryakov from comment #13) > > I’m surprised that not having a result didn’t cause the EWS to fail. > > Please feel encouraged to file a bug! I think that's a pretty bad issue. Ok. https://bugs.webkit.org/show_bug.cgi?id=177723.
Comment on attachment 322321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322321&action=review > Source/WebCore/xml/XMLHttpRequest.cpp:175 > + ASSERT(scriptExecutionContext()->isDocument()); I would write: ASSERT(is<Document>(scriptExecutionContext())); But I see the file already has three places that do it the other way, calling isDocument(). > Source/WebCore/xml/XMLHttpRequest.cpp:257 > + if (!scriptExecutionContext()->isDocument() && type == ResponseType::Document) > + return { }; As I said above, I like the is<Document> style better. If you wrote it is<Document>(scriptExecutionContext() then you would include an unnecessary null check. But it’s annoying that the assumption that script execution context is non-null is implicit; when called from script I guess that is sort of obvious, but on the other hand it’s annoying that it’s called “script execution context” yet we stash it when created and don’t get a fresh one when being called from script again. I kind of wish there was a version that returned a reference and asserted.
Comment on attachment 322321 [details] Patch Clearing flags on attachment: 322321 Committed r222690: <http://trac.webkit.org/changeset/222690>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34761304>
Add missing test results in r222693.