137428 - [FIX]broken layout in dynamically (javascript) built div table since nightly builds after 0.9.9 release (cloneNode does not copy style attribute)

Status: VERIFIED FIXED

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

Description

[Philip Timmermann]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; de-AT; rv:0.9.9+)
Gecko/20020413
BuildID:    2002041308

A made some little small Javascript code which builds a table made of div's and
puts numbers (made of gif's) into certain cells...

It's located here: www.pepto.de/test/div.html

It works beautifully in Mozilla 0.9.9 (and probably versions before that) and
Internet Explorer 5.1.3 on MacOS X (although IE is pretty slow at it)...

I downloaded the latest nightly build a few days ago and today again and
recognized that my little small script doesn't seem to work any longer in the
nighties after 0.9.9... The numbers are all over the screen vertically and stuff
like that...

I guess somebody changed something in the Mozilla-code and now it's broken...
I'm no Javascript-wizard, but I'm not doing anything too fancy in my script so I
think that this is a bug in Mozilla (especially as it worked in Mozilla before
and works in IE)...

I'm sorry if I'm the one to blame as my JS-programming skills are far from
perfect... =)


Reproducible: Always
Steps to Reproduce:
1. Load www.pepto.de/test/div.html in Mozilla 0.9.9 release
2. Load www.pepto.de/test/div.html in Mozilla latest nightly build
3. See the difference and try to fix the bug ;-)

Actual Results:  The numbers are all over the screen vertically and stuff like
that...

Expected Results:  Put the numbers correctly in certain div's like Moz 0.9.9 and
IE do...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

It looks like cloneNode loses the "style" attribute....  This is a regression
from 0.9.9.

Fabian?  Any ideas?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Oh, backing out the patch from bug 26528 fixes this.  So it's something in that
general area of things...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to fix

The problem is that nsGenericHTMLElement::SetAttr doesn't parse style
attributes for things that are not in a document.  So we need to copy over the
HTMLValue of the style attr instead of setting it as a string in cloneNode().

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Same patch as diff -uw

This may be a little easier to review.	The easiest way to review the patch is
probably to apply it and look at the code, though.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, and the patch does not regress bug 26528

Comment 7

[Johnny Stenback (:jst)]

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

+      if (name == nsHTMLAtoms::style) {

Make that "name == nsHTMLAtoms::style && namespace == kNameSpaceID_None".

sr=jst

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: same patch with namespace check

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 79285 [details] [diff] [review]
same patch with namespace check

marking jst's sr

Comment 10

[Fabian Guisset]

Comment on attachment 79285 [details] [diff] [review]
same patch with namespace check

eww, this one escaped my vigileance! thanks!
r=fabian

Comment 11

[Boris Zbarsky [:bzbarsky]]

taking to land

Comment 12

[Boris Zbarsky [:bzbarsky]]

Checked in on the trunk.  I'll ask for driver approval for the branch tomorrow
once I can say that there have been no regressions filed for a bit....

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 79285 [details] [diff] [review]
same patch with namespace check

a=[e-mail] fro branch checkin

Comment 14

[Boris Zbarsky [:bzbarsky]]

fixed on branch.

Comment 15

[Boris Zbarsky [:bzbarsky]]

*** Bug 140189 has been marked as a duplicate of this bug. ***

Comment 16

[Prashant Desale]

verified1.0.0 on 05-22-18-1.0.0 & also Verified on trunk on 05-23-08-trunk.