1184647 - cloned image node omits naturalWidth and naturalHeight properties

Status: RESOLVED FIXED

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

Description

[scaturrob]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.134 Safari/537.36

Steps to reproduce:

I created a fiddle that demonstrates the issue:

http://jsfiddle.net/scaturrob/yqed6ts0/


Actual results:

The loaded image returns the correct dimensions for naturalWidth and naturalHeight. Cloning that image results in a 0 value for both naturalWidth and naturalHeight on the clone.


Expected results:

The original node and clone should have identical naturalWidth and naturalHeight properties

Comment 1

[Boris Zbarsky [:bzbarsky]]

Nice catch.

The problem is that cloning passes false for aNotify to SetAttr, so the image load doesn't get started on the clone.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Cloning an image should ensure that the clone tries to load the src, if any

Olli, sorry to pile onto your review load, but this needs some checking wrt the static document bits and the responsive image interaction, and I seem to recall you know about the latter.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8635413 [details] [diff] [review]
Cloning an image should ensure that the clone tries to load the src, if any

oh, hmm, I think we may want to pass true as aNotify in CopyInnerTo, but that needs some more investigation, so not in this bug.
2 spaces for indentation.

but I'm not familiar with responsive image interaction. jdm has done some work there.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8635413 [details] [diff] [review]
Cloning an image should ensure that the clone tries to load the src, if any

But for static document case, yes, we don't want to load anything there, but just
clone.

Comment 5

[Josh Matthews [:jdm]]

Comment on attachment 8635413 [details] [diff] [review]
Cloning an image should ensure that the clone tries to load the src, if any

Review of attachment 8635413 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a73c096a3d

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e0a73c096a3d