Bug 121695 – REGRESSION (r156140): Srcset tests are frequently crashing

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Alexey Proskuryakov]

I think that URL bytes need to be copied now.

[Romain Perier]

I confirm the issue, I have a patch locally, the URL assignment
Must be copied using String::substring. It solves
The problem. Could you assign me this bug please ? Thanks in advance

[Romain Perier]

It should be fixed this week end, or feel free
To fix it yourself if this is really important :)

[Alexey Proskuryakov]

Created attachment 212196 [details]
proposed fix

[Alexey Proskuryakov]

Yes, it's fairly urgent. I want bots to be very very green.

Also made a bunch of style fixes to make this code better align with WebKit style.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r156183>.

[Darin Adler]

Comment on attachment 212196 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=212196&action=review

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:393
> +        image.imageURL = String(srcsetAttribute.characters() + imageURLStart, imageURLEnd - imageURLStart);

Might be clearer to use the substring function to do this. Even possibly leaves the door open for future optimization where we share the character backing store between the original string and the substring.

[Darin Adler]

Comment on attachment 212196 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=212196&action=review

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:384
> +                imageScaleFactor = charactersToFloat(srcsetAttribute.characters() + imageScaleStart, scaleFactorLengthWithoutUnit, &validScaleFactor);

A shame that this forces an 8-bit string to be converted to 16-bit. Not new to this patch, but unpleasant.