Bug 118702 – XSLStyleSheet::parseString shouldn't upconvert 8-bit string

• Status: NEW
• Component: New Bugs

[Ryosuke Niwa]

XSLStyleSheet::parseString shouldn't upconvert 8-bit string

[Ryosuke Niwa]

Created attachment 206720 [details]
Fixes the bug

[Geoffrey Garen]

Comment on attachment 206720 [details]
Fixes the bug

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

> Source/WebCore/xml/XSLStyleSheetLibxslt.cpp:153
> +    const bool is8Bit = string.is8Bit();
> +    const char* characters = is8Bit ? reinterpret_cast<const char*>(string.characters8()) : reinterpret_cast<const char*>(string.characters16());

We don't usually use const for local variables like this.

> Source/WebCore/xml/parser/XMLDocumentParser.h:226
> +    const unsigned char BOMHighByte = *reinterpret_cast<const unsigned char*>(&BOM);

I know you didn't write this function, but it's very weird to call this value the "high byte", since it might be the high or low byte depending on endianness, and it's actually the low byte on 99% of computers these days.

I'd call this highOrLowByte.

[Ryosuke Niwa]

(In reply to comment #2)
> > Source/WebCore/xml/parser/XMLDocumentParser.h:226
> > +    const unsigned char BOMHighByte = *reinterpret_cast<const unsigned char*>(&BOM);
> 
> I know you didn't write this function, but it's very weird to call this value the "high byte", since it might be the high or low byte depending on endianness, and it's actually the low byte on 99% of computers these days.
> 
> I'd call this highOrLowByte.

How about firstByte?

[Build Bot]

Comment on attachment 206720 [details]
Fixes the bug

Attachment 206720 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1081122

New failing tests:
http/tests/security/cross-origin-xsl-redirect-BLOCKED.html
dom/xhtml/level3/core/canonicalform09.xhtml
fast/xsl/xslt-bad-import-uri.html
http/tests/misc/xslt-bad-import.html
dom/xhtml/level3/core/canonicalform08.xhtml

[Build Bot]

Created attachment 206743 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 206720 [details]
Fixes the bug

Attachment 206720 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1088236

New failing tests:
dom/xhtml/level3/core/canonicalform09.xhtml
fast/xsl/xslt-bad-import-uri.html
http/tests/misc/xslt-bad-import.html
http/tests/security/cross-origin-xsl-redirect-BLOCKED.html
editing/pasteboard/copy-image-with-alt-text.html
dom/xhtml/level3/core/canonicalform08.xhtml
http/tests/inspector/inspect-element.html

[Build Bot]

Created attachment 206797 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Geoffrey Garen]

> How about firstByte?

Sounds good.

Tests seem angry :(.