1142478 - rowspan="02" does not round-trip cleanly, gets corrupted to rowspan="2"

Status: VERIFIED FIXED

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

Description

[Roan Kattouw]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150306140254

Steps to reproduce:

In a console, run:

div = document.createElement('div');
div.innerHTML = '<table><tr><td rowspan="02"></td></tr></table>';
div.innerHTML;


Actual results:

"<table><tbody><tr><td rowspan="2"></td></tr></tbody></table>"


Expected results:

"<table><tbody><tr><td rowspan="02"></td></tr></tbody></table>"

Comment 1

[Roan Kattouw]

Note that this isn't a serialization issue, the attribute corruption occurs on parse:

> div.querySelector('td').getAttribute('rowspan')
"2"

This is even worse because it means there's no way to find out what the original value of the attribute was.

Chrome does not have this bug. IE does have this bug, and it also performs many other normalizations, like width="230px"->width="230", color="Red"->color="red", bgcolor="#FFDEAD"->bgcolor="#ffdead", style="color:#ffd"->style="color: rgb(255, 255, 221);" and even style="invalid-css"->style="". Firefox fortunately doesn't perform any of these as far as I can tell.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Right, the problem is that nsContentUtils::ParseHTMLInteger doesn't detect this as a case in which serialization would produce a value different from the input.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix integer attribute parsing to not lose track of leading zeroes

Comment 4

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

Comment on attachment 8576687 [details] [diff] [review]
Fix integer attribute parsing to not lose track of leading zeroes

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

::: dom/base/nsContentUtils.cpp
@@ +989,5 @@
> +  CheckedInt32 value = 0;
> +
> +  // Check for leading zeros first.
> +  bool hadLeadingZero = false;
> +  bool hadMultipleLeadingZeros = false;

Might be easier to simply count the leading zeros, rather than deal with two state variables.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Might be easier to simply count the leading zeros

I considered that, but then the paranoid in me didn't want to deal with that count maybe overflowing.  I guess I could have used a 64-bit counter...  I'll do that.

Comment 6

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83ac7b782760 and then https://hg.mozilla.org/integration/mozilla-inbound/rev/35690b2e6d73 to fix issues with getting the value of an invalid CheckedInt and an existing bogus test that required the wrong behavior.

Comment 7

[Boris Zbarsky [:bzbarsky]]

And https://hg.mozilla.org/integration/mozilla-inbound/rev/cbb20d85f7fe for the web-platform-tests bits.

Comment 8

[Boris Zbarsky [:bzbarsky]]

And https://hg.mozilla.org/integration/mozilla-inbound/rev/dd04d7c32b67 for yet another missed wpt annotation.  I wish it were less painful to update those annotation files.  :(

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83ac7b782760
https://hg.mozilla.org/mozilla-central/rev/35690b2e6d73
https://hg.mozilla.org/mozilla-central/rev/cbb20d85f7fe
https://hg.mozilla.org/mozilla-central/rev/dd04d7c32b67

Comment 10

[Ashickur Rahman]

Successfully reproduce this bug on Firefox 38.0.5 Build ID 20150525141253, User Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Firefox/38.0

The fix works for me on Firefox 39.0, Build ID: 	20150604162752, User Agent: Mozilla/5.0 (Windows NT 6.1; rv:39.0) Gecko/20100101 Firefox/39.0