<!doctype html> <span id="inline" style="width: 10%">A</span> <pre> <script> document.writeln(getComputedStyle(inline).width); </script> </pre> All engines but WebKit show "10%" instead of "auto".
Created attachment 370191 [details] To check ews result
Comment on attachment 370191 [details] To check ews result Attachment 370191 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12225010 New failing tests: legacy-animation-engine/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html
Created attachment 370192 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 370191 [details] To check ews result Attachment 370191 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12225030 New failing tests: legacy-animation-engine/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html
Created attachment 370193 [details] Archive of layout-test-results from ews106 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 370191 [details] To check ews result Attachment 370191 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12225104 New failing tests: fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html
Created attachment 370194 [details] Archive of layout-test-results from ews210 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews210 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 370191 [details] To check ews result Attachment 370191 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12225059 New failing tests: legacy-animation-engine/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html
Created attachment 370195 [details] Archive of layout-test-results from ews116 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 370191 [details] To check ews result Attachment 370191 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12225073 New failing tests: legacy-animation-engine/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html
Created attachment 370196 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.5
Created attachment 370205 [details] Update LayoutTest
Created attachment 370210 [details] Patch
Created attachment 370211 [details] Patch
Comment on attachment 370211 [details] Patch Attachment 370211 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12227288 New failing tests: fast/media/mq-any-hover-styling.html fast/text/crash-font-family-parsed.html fast/css-generated-content/pseudo-animation.html fast/css/getComputedStyle/getComputedStyle-length-unit.html imported/w3c/web-platform-tests/css/cssom/getComputedStyle-pseudo.html legacy-animation-engine/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/css/getComputedStyle/zoom-on-display-none.html inspector/css/pseudo-element-matches.html fast/dom/length-attribute-mapping.html fast/css/computed-width-without-renderer.html imported/w3c/web-platform-tests/infrastructure/assumptions/html-elements.html fast/css/getComputedStyle/getComputedStyle-zoom-and-background-size.html fast/media/mq-any-pointer-styling.html fast/media/mq-hover-styling.html fast/css/getComputedStyle/getComputedStyle-resolved-values.html fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html fast/history/visited-generated-content-test.html fast/css/getComputedStyle/pending-stylesheet.html fast/media/mq-pointer-styling.html fast/css/getComputedStyle/computed-style-without-renderer.html media/modern-media-controls/tracks-support/tracks-support-captions-offset-with-controls-bar.html jquery/css.html
Created attachment 370212 [details] Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 370213 [details] Patch
Created attachment 370214 [details] Patch
Created attachment 370217 [details] Update legacy-animation-engine layout test
Could you please review this change?
Comment on attachment 370217 [details] Update legacy-animation-engine layout test View in context: https://bugs.webkit.org/attachment.cgi?id=370217&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3190 > + if ((renderer && style.display() != DisplayType::Contents) && !renderer->isRenderSVGModelObject()) { How can you end up with a box but be display: contents? That check seems wrong / unnecessary. Probably just: if (renderer && !isNonReplacedInline(*renderer) && !renderer->isRenderSVGModelObject()) return zoomAdjustedPixelValue(sizingBox(*renderer).height(), style); return zoomAdjustedPixelValueForLength(style.height(), style);
(disclaimer: I'm not a WebKit reviewer)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #21) > Comment on attachment 370217 [details] > Update legacy-animation-engine layout test > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370217&action=review > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3190 > > + if ((renderer && style.display() != DisplayType::Contents) && !renderer->isRenderSVGModelObject()) { > > How can you end up with a box but be display: contents? That check seems > wrong / unnecessary. Probably just: > > if (renderer && !isNonReplacedInline(*renderer) && > !renderer->isRenderSVGModelObject()) > return zoomAdjustedPixelValue(sizingBox(*renderer).height(), style); > return zoomAdjustedPixelValueForLength(style.height(), style); That's right. The element with display: contents should disappear so there will be no such render object, means that the check is unnecessary/wrong. As you suggested I will remove the display:content condition check. Thank you for your review, Emilio:)
Created attachment 370221 [details] Patch
Comment on attachment 370221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370221&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3190 > if (renderer && !renderer->isRenderSVGModelObject()) { ^^^ not part of your patch, but just realized this function name is misspelled. "in" should have capital I. Does not need to fixed now. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3191 > // According to http://www.w3.org/TR/CSS2/visudet.html#the-height-property, Is this comment still applicable then. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3192 > // the "height" property does not apply for non-replaced inline elements. ? Also, is the code change correct for when a non-replaced inline has an explicit height: auto? Anyway, not a css expert, yet. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3513 > // According to http://www.w3.org/TR/CSS2/visudet.html#the-width-property, Is comment still applicable?
Comment on attachment 370221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370221&action=review > Source/WebCore/ChangeLog:13 > + 'If the property applies to the element or pseudo-element This seems to contradict the spec undefined nature for width and height of of non-replaced inline elements in CSS 2.2
Comment on attachment 370221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370221&action=review >> Source/WebCore/ChangeLog:13 >> + 'If the property applies to the element or pseudo-element > > This seems to contradict the spec undefined nature for width and height of of non-replaced inline elements in CSS 2.2 This spec(https://drafts.csswg.org/cssom/#resolved-value) section's statement is for many properties e.g. block-size/height/inline-size/margin-block-end/... So, widht/height is belong to "Otherwise" case in below. >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3190 >> if (renderer && !renderer->isRenderSVGModelObject()) { > > ^^^ not part of your patch, but just realized this function name is misspelled. "in" should have capital I. Does not need to fixed now. It is just typo, so I will fix it with an separate unreviewed patch. >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3191 >> // According to http://www.w3.org/TR/CSS2/visudet.html#the-height-property, > > Is this comment still applicable then. When I see even the draft version of this spec(https://drafts.csswg.org/css-sizing-3/#preferred-size-properties), it's still applicable. It says about width/height, "Applies to: all elements except non-replaced inlines" >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3513 >> // According to http://www.w3.org/TR/CSS2/visudet.html#the-width-property, > > Is comment still applicable? ditto.
Comment on attachment 370221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370221&action=review >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3192 >> // the "height" property does not apply for non-replaced inline elements. > > ? Also, is the code change correct for when a non-replaced inline has an explicit height: auto? Anyway, not a css expert, yet. I missed out to add comment here in the previous comments. The spec, https://drafts.csswg.org/cssom/#resolved-value says that width and height should return the computed value, so if 'auto' is specified, then it should return 'auto'. So this change is correct, I think.
Cool
Hi, Could this patch get a review mark?
Comment on attachment 370221 [details] Patch Rejecting attachment 370221 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 370221, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/12278678
Created attachment 370585 [details] Patch
Comment on attachment 370585 [details] Patch Clearing flags on attachment: 370585 Committed r245768: <https://trac.webkit.org/changeset/245768>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51129484>