HTMLImageElement.width / height attributes should be unsigned as per the HTML specification: - https://html.spec.whatwg.org/#htmlimageelement However, they are signed in WebKit. Firefox agrees with the specification.
Created attachment 288238 [details] Patch
Comment on attachment 288238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288238&action=review > Source/WebCore/html/HTMLImageElement.cpp:408 > + Optional<int> height = parseHTMLNonNegativeInteger(attributeWithoutSynchronization(heightAttr)); auto > Source/WebCore/html/HTMLImageElement.h:42 > - static Ref<HTMLImageElement> createForJSConstructor(Document&, const int* optionalWidth, const int* optionalHeight); > + static Ref<HTMLImageElement> createForJSConstructor(Document&, Optional<unsigned> width, Optional<unsigned> optionalHeight); optionalHeight could just be height.
Comment on attachment 288238 [details] Patch Attachment 288238 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2031121 New failing tests: imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
Created attachment 288246 [details] Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Created attachment 288272 [details] Patch
Comment on attachment 288238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288238&action=review >> Source/WebCore/html/HTMLImageElement.cpp:408 >> + Optional<int> height = parseHTMLNonNegativeInteger(attributeWithoutSynchronization(heightAttr)); > > auto I actually do not like using auto for Optional<> type because I find the resulting code a bit confusing: auto height = parseHTMLNonNegativeInteger(attributeWithoutSynchronization(heightAttr)); if (height) // ... It makes it look like you're checking if height is != 0, which is not the case. One way to work around this would be to keep 'optional' in the variable name: auto optionalHeight = parseHTMLNonNegativeInteger(attributeWithoutSynchronization(heightAttr)); if (optionalHeight) // ... But I don't think it is better than using an explicit Optional<> type and a shorter 'height' variable name. >> Source/WebCore/html/HTMLImageElement.h:42 >> + static Ref<HTMLImageElement> createForJSConstructor(Document&, Optional<unsigned> width, Optional<unsigned> optionalHeight); > > optionalHeight could just be height. Oops, fixed.
Comment on attachment 288272 [details] Patch Clearing flags on attachment: 288272 Committed r205655: <http://trac.webkit.org/changeset/205655>
All reviewed patches have been landed. Closing bug.