Closed Bug 251354 Opened 21 years ago Closed 20 years ago

[FIXr]When loading a fragment from DOMParser, images do not display.

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8beta2

People

(Reporter: anko.com+bugzilla, Assigned: bzbarsky)

References

Details

Attachments

(4 files, 1 obsolete file)

Test case for this bug
5.67 KB, application/zip
Details
Testcase that can just be run directly
868 bytes, application/xhtml+xml
Details
Same but with makefile change too
20.09 KB, patch
sicking
: review+
peterv
: superreview+
asa
: approval1.8b2+
Details | Diff | Splinter Review
Tiny followup patch
1.15 KB, patch
sicking
: review+
shaver
: superreview+
chofmann
: approval1.8b2+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040709 Firefox/0.9.2 (MOOX-AV) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040709 Firefox/0.9.2 (MOOX-AV) When a document fragment containing an img tag is parsed by the DOMParser and inserted somewhere in the document, the image does not show but the alt-text does instead! eg. (please forgive this paste but this is my first bug entry and i can't find out how to attach a file). <?xml version="1.0"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <body onload="read();"> <script type="text/javascript"> <![CDATA[ function read() { var output = "<div xmlns='http://www.w3.org/1999/xhtml' id='sceneNode'><img src='test.jpg' alt='test failed'/></div>"; try { var sceneContainer = document.getElementById("scenecontainer"); var sceneNode = document.getElementById("sceneNode"); var parser=new DOMParser(); var resultDoc=parser.parseFromString(output,"text/xml"); sceneContainer.replaceChild(resultDoc.documentElement, sceneNode); } catch (e) { alert(e); } } ]]> </script> <div id="scenecontainer"> <div id="sceneNode" class="container"> </div> </div> </body> </html> Reproducible: Always Steps to Reproduce: 1. put an image in the directory with the page code from the details box and name it test.jpg (any image will do). 2. view the page Actual Results: test failed will be displayed Expected Results: test.jpg should be displayed This bug has appeared in every release i've tested.
Attached file Test case for this bug —
This is a test case for this bug - just run the testbug.xhtml file included.
Attachment #153143 - Attachment filename: render bugs.zip → renderbugs.zip
layout:images is probably a better component, although it could well be a DOM bug instead
Component: Image: GFX → Layout: Images
QA Contact: core.layout.images
Stylesheets don't work either. The issue is that DOMParser loads the XML "as data", so image loading, script loading, stylesheet loading, etc is disabled for the things in the fragment (see nsXMLContentSink::CreateElement). They are never reenabled, so when you insert the subtree into the "real" document the image loads (style loads, script loads) do not start. Now we definitely don't want to start the loads while doing the DOMParser thing. I don't even think we want to reenable once we're done parsing the node, since manipulation of the "as data" DOM should not start loads. So _maybe_ we should unset the disabled state when the nodes are moved to a different document? Perhaps when we reparent the content wrapper?
Seems like importNode() should set the bit to be the same as the document it is being imported into, yeah.
ImportNode is actually rather sad -- it's just a cloneNode. It doesn't change the ownerDocument! See bug 251025. Once that's fixed, hopefully it will just hook directly into the content-wrapper reparenting code...
::SetDocument would seem to be the right place to do it. A different approach would be to not let the elements in question keep track of this, but rather let the elements ask the document whenever they needed the info. Redundant information is always a bad idea.
Hmm.. There may be times when you want to disable a particular element in a document that generally allows resource loading. Stylesheets certainly do that.
Maybe we should use different mechanisms for disabling single resources and disabling resource-loading in a document. Otherwise we will reenable all stylesheets when they are moved from a data-document to a displayed document.
Isn't it desirable to reenable them in that case?
dunno. I could imagine a senario where the user wants to load a fragment and then move it to a displayed document. But before disable a few resources because they're not being used yet. OTOH, i could also imagine a senario where you'd want to load a document as fragment, then enable a stylesheet and play around in the stylesheets DOM.
1) Can someone please confirm this bug 2) Should we set it to depending on bug 251025 (document.importNode) 3) Should the component be moved to somewhere in the DOM as it seems to me it's a DOM bug (if script/stylesheets don't work either). 4) Are there any exposed XPCOM interfaces to force the image to load as a workaround? are there any other workarounds? Although I can't find anything in css2, is there a way to specify an img's src via css?
> Are there any exposed XPCOM interfaces to force the image to load as a > workaround? Exposed to priveleged code (chrome, c++, etc), yes. See nsIImageLoadingContent. You can use that to reenable the image. If you do that before inserting the fragment into the new document, it'll load.
Assignee: jdunn → general
Status: UNCONFIRMED → NEW
Component: Layout: Images → DOM
Ever confirmed: true
QA Contact: core.layout.images → ian
*** Bug 260957 has been marked as a duplicate of this bug. ***
I think we should definitely decouple disabling on the element from disabling because the document is loaded as data. The latter could be pushed into an nsIContentPolicy impl. All loads already go through that anyway. And since the document has a member for whether it's loaded as data, we could just push that boolean out through a new accessor on nsIDocument. Seem reasonable?
Which, of course, is blocked by bug 203211... XSLT needs to look at content policy.
Depends on: 203211
Attached patch Patch (obsolete) — — Splinter Review
Comment on attachment 161674 [details] [diff] [review] Patch Summary of patch: 1) Expose mLoadedAsData 2) Differentiate between that and mLoadedAsInteractiveData, since we don't want to block stylesheet/script loads for XBL. 3) Remove the check for images it the content sink and replace with content policy. Stylesheets/scripts are already ok for documents loaded as data, since we just disable the CSSLoader and ScriptLoader instead of disabling the individual nodes.
Attachment #161674 - Flags: superreview?(peterv)
Attachment #161674 - Flags: review?(bugmail)
Attachment #161674 - Attachment is obsolete: true
Attachment #161674 - Flags: superreview?(peterv)
Attachment #161674 - Flags: superreview-
Attachment #161674 - Flags: review?(bugmail)
Attachment #161674 - Flags: review-
Attachment #161675 - Flags: superreview?(peterv)
Attachment #161675 - Flags: review?(bugmail)
Attachment #161675 - Flags: superreview?(peterv) → superreview+
Taking, so I stop losing track of this.... Sicking, any chance of getting to this in time for 1.8b3?
Assignee: general → bzbarsky
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Summary: When loading a fragment from DOMParser, images do not display. → [FIX]When loading a fragment from DOMParser, images do not display.
Target Milestone: --- → mozilla1.8beta3
Yeah, i'll try to do it this weekend
Blocks: 289714
Comment on attachment 161675 [details] [diff] [review] Same but with makefile change too Yay, this was simple, sorry it took so long :(
Attachment #161675 - Flags: review?(bugmail) → review+
Comment on attachment 161675 [details] [diff] [review] Same but with makefile change too Requesting 1.8b2 approval... I think we want this for 1.8 (if nothing else, jst wants bug 289714, which depends on this for 1.8), and the more testing this gets the better. This is a quite safe patch, so I think it should be ok for b2.
Attachment #161675 - Flags: approval1.8b2?
Summary: [FIX]When loading a fragment from DOMParser, images do not display. → [FIXr]When loading a fragment from DOMParser, images do not display.
Comment on attachment 161675 [details] [diff] [review] Same but with makefile change too a=asa
Attachment #161675 - Flags: approval1.8b2? → approval1.8b2+
Patch checked in. It doesn't quite fix the bug in current builds, though, because images now don't load themselves on being inserted into a document if that doesn't change their URI. We should probably be bypassing that when we have a blocked image.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta3 → mozilla1.8beta2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
shaver, can you sr? I'm afraid I've piled way too much stuff in jst's queue...
Attachment #180342 - Flags: superreview?(shaver)
Attachment #180342 - Flags: review?(bugmail)
Comment on attachment 180342 [details] [diff] [review] Tiny followup patch sr=shaver
Attachment #180342 - Flags: superreview?(shaver) → superreview+
Comment on attachment 180342 [details] [diff] [review] Tiny followup patch Could this please be approved for 1.8b2? All this does is to try loading the image again when an image that was blocked by preferences or security is moved to a different document...
Attachment #180342 - Flags: approval1.8b2?
Comment on attachment 180342 [details] [diff] [review] Tiny followup patch a=chofmann
Attachment #180342 - Flags: approval1.8b2? → approval1.8b2+
Fixed.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
There is a workaround. The workaround for this issue is to use innerHTML. One might think, but that doesn’t work in XHTML/XML mode! Well, it does, when used on an element that has been dynamically created. The following example illustrates this: var xml = document.createElement('div'); xml.innerHTML = 'test <img src="kangaroo.png" />'; this.appendChild(xml); I hope this information is of some use to people who run into this bug :). ~Grauw
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: