459470 - Need to set documentURI and baseURI correctly for XHR documents

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

HR documents are created with the wrong documentURI.

It doesn't actually look like we ever expose documents of the wrong URI anywhere, but the live internally as such while loading sometimes which seems risky.

It also seems like a waste of cycles to create the document when not needed.

Patch to fix attached

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 342683 [details] [diff] [review]
Patch to fix

Actually, we can delay document creation even more. Will write new patch

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 342683 [details] [diff] [review]
Patch to fix

Ok, leaving as is.

What we could do is to move the parsing into GetResponseXML. However that would mean parsing the whole document when the script gets .responseXML which might take a while. Bz measured 5 sec to parse a 13MB doc. Seems bad to freeze the UI for so long time so lets stick with the current strategy for now.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ugh, left some dump() statements in the testfile. Please ignore those.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 342683 [details] [diff] [review]
Patch to fix

>+++ b/content/base/src/nsXMLHttpRequest.cpp
>+    // Setting the base URI to |uri| won't work if the document has a null

s/|uri|/|baseURI/

With that and the dump()s removed, looks good.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in: http://hg.mozilla.org/mozilla-central/rev/ee5135f3f773

Comment 6

[Boris Zbarsky [:bzbarsky]]

Weshould take this on 1.9.0, since that would fix bug 474211 there.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Branch patch

Porting was really easy so if you don't get to review this I feel confident enough to just check in. All I did was remove the parts that did
s/mDocument/mResponseXML/, and removed code and tests to deal with xs-xhr.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 359877 [details] [diff] [review]
Branch patch

Looks good.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 359877 [details] [diff] [review]
Branch patch

Approved for 1.9.0.7, a=dveditz for release-drivers.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

The checked in tests in the patch pass. Is there any manual way to verify this fix?

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

No, that's it

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Marking as verified then for 1.9.0.7.