This bug has been already FIXED in Blink, see the patch at the following CL: https://codereview.chromium.org/2523573003/ I'll try to port the same solution in WebKit. Created attachment 305588 [details]
Patch
Comment on attachment 305588 [details] Patch Attachment 305588 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3427041 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical.html Created attachment 305591 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305588 [details] Patch Attachment 305588 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3427042 New failing tests: fast/inline-block/baseline-vertical.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/writing-mode/border-styles-vertical-lr.html Created attachment 305592 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 305588 [details] Patch Attachment 305588 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3427038 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical.html Created attachment 305593 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305588 [details] Patch Attachment 305588 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3427430 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical.html fast/backgrounds/background-leakage-transforms.html Created attachment 305598 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 305734 [details]
Patch
Comment on attachment 305734 [details] Patch Attachment 305734 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3434084 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical.html Created attachment 305738 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305734 [details] Patch Attachment 305734 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3434089 New failing tests: fast/inline-block/baseline-vertical.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/writing-mode/border-styles-vertical-lr.html Created attachment 305741 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 305734 [details] Patch Attachment 305734 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3434086 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/text/emphasis-avoid-ruby.html fast/text/emphasis-overlap.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical.html Created attachment 305744 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305734 [details] Patch Attachment 305734 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3434099 New failing tests: fast/inline-block/baseline-vertical.html Created attachment 305748 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 305785 [details]
Patch
Comment on attachment 305785 [details] Patch Attachment 305785 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3435858 New failing tests: fast/inline-block/baseline-vertical.html Created attachment 305797 [details]
Archive of layout-test-results from ews102 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305785 [details] Patch Attachment 305785 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3435991 New failing tests: fast/inline-block/baseline-vertical.html Created attachment 305799 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 305785 [details] Patch Attachment 305785 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3435970 New failing tests: fast/inline-block/baseline-vertical.html Created attachment 305800 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 305802 [details]
Patch
Comment on attachment 305802 [details] Patch Attachment 305802 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3436394 New failing tests: fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical.html fast/backgrounds/background-leakage-transforms.html Created attachment 305810 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 305887 [details]
Patch
Created attachment 305913 [details]
Patch
Comment on attachment 305913 [details] Patch Attachment 305913 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3443354 New failing tests: fast/inline-block/baseline-vertical.html Created attachment 305925 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 306011 [details]
Patch
@dhyatt, could you please take a look ? Gentle ping, reviewers. (In reply to Javier Fernandez from comment #36) > Gentle ping, reviewers. Is the latest patch the one for review? It doesn't have the right flags on it, so I don't think anyone saw it. Given how long it's been, it also may require a rebaseline. (In reply to Jon Lee from comment #37) > (In reply to Javier Fernandez from comment #36) > > Gentle ping, reviewers. > > Is the latest patch the one for review? It doesn't have the right flags on > it, so I don't think anyone saw it. Given how long it's been, it also may > require a rebaseline. Yes, it is. Sorry about the missing review flag :( @zalan @hyatt, could you please provide some feedback on this patch ? Thanks Comment on attachment 306011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306011&action=review Zalan/Hyatt should review. > LayoutTests/fast/inline-block/baseline-vertical-expected.txt:4 > +layer at (0,0) size 800x600 > + RenderView at (0,0) size 800x600 > +layer at (0,0) size 800x503 > + RenderBlock {HTML} at (0,0) size 800x504 Can you make a refest instead? Render tree dumps are fragile and annoying. *reftest (In reply to Antti Koivisto from comment #40) > Comment on attachment 306011 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=306011&action=review > > Zalan/Hyatt should review. Yes, I supposed it. > > > LayoutTests/fast/inline-block/baseline-vertical-expected.txt:4 > > +layer at (0,0) size 800x600 > > + RenderView at (0,0) size 800x600 > > +layer at (0,0) size 800x503 > > + RenderBlock {HTML} at (0,0) size 800x504 > > Can you make a refest instead? Render tree dumps are fragile and annoying. Well, I don't like at all the tree dumps and image tests. However, for this baseline alignment cases I wasn't able to make a valid reference tests. Many of these baseline tests are implemented using tree dumps as well. In some cases they use flexbox vs block to define reference tests, but in this case both layouts use the same code, so we wouldn't notice regressions. The problem with baseline test is that it highly depends on the font family and size. I could have used 'ahem' but then it'd use a different baseline (central if I remember correctly) so we don't verify completely the changes applied in the code. I thought about using absolute positioned for the reference tests, but as I said, it may fail because of different font family and size when running it in different platforms. If anybody has an idea, I'd be happy to define a reference tests instead, of course. Created attachment 328963 [details]
Patch
Replace pixel tests by reference tests.
Comment on attachment 328963 [details] Patch Attachment 328963 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5614393 New failing tests: fast/inline-block/baseline-vertical-08.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-06.html fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical-07.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-05.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 328966 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328963 [details] Patch Attachment 328963 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5614420 New failing tests: fast/inline-block/baseline-vertical-08.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-06.html fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical-07.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-05.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 328968 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328963 [details] Patch Attachment 328963 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5614447 New failing tests: fast/inline-block/baseline-vertical-08.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-06.html fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical-07.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-05.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 328970 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 328963 [details] Patch Attachment 328963 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5614432 New failing tests: fast/inline-block/baseline-vertical-08.html fast/backgrounds/background-leakage-transforms.html fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-06.html fast/writing-mode/border-styles-vertical-lr.html fast/inline-block/baseline-vertical-07.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-05.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 328972 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 329053 [details]
Patch
Comment on attachment 329053 [details] Patch Attachment 329053 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5623432 New failing tests: fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 329063 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 330898 [details]
Patch
Comment on attachment 330898 [details] Patch Attachment 330898 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6020397 New failing tests: fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-04.html fast/inline-block/baseline-vertical-03.html Created attachment 330908 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 330898 [details] Patch Attachment 330898 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6024877 New failing tests: http/tests/misc/slow-loading-animated-image.html Created attachment 330959 [details]
Archive of layout-test-results from ews100 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 331356 [details]
Patch
Added Failure TestExppectations for ios-sim
Comment on attachment 331356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331356&action=review The patch LGTM. Just a question about border-styles-vertical-lr.html test, for the new expected files I see color changes in the borders, is that expected? > Source/WebCore/rendering/RenderBlockFlow.cpp:3061 > + // InlineFlowBox::placeBoxesInBlockDirection will flip lines in > + // case of verticalLR mode, so we can assume verticalRL for now. Nit: This could be just one line. > LayoutTests/ChangeLog:30 > + * fast/text/emphasis-overlap-expected.txt: Nit: I believe this ChangeLog is not accurate. > LayoutTests/platform/ios-simulator/TestExpectations:59 > +# These tests seem to fail due pixel rounding errors in the absolute positioned elements used as reference. Nit: Shouldn't we explain that we're skipping these in the ChangeLog. Comment on attachment 331356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331356&action=review >> Source/WebCore/rendering/RenderBlockFlow.cpp:3061 >> + // case of verticalLR mode, so we can assume verticalRL for now. > > Nit: This could be just one line. ok >> LayoutTests/ChangeLog:30 >> + * fast/text/emphasis-overlap-expected.txt: > > Nit: I believe this ChangeLog is not accurate. Yes, it seems some files are missing from the list. >> LayoutTests/platform/ios-simulator/TestExpectations:59 >> +# These tests seem to fail due pixel rounding errors in the absolute positioned elements used as reference. > > Nit: Shouldn't we explain that we're skipping these in the ChangeLog. ok. It'd be also good to have a bug report to point to, but I wasn't able to find a simple test case to reproduce it. (In reply to Manuel Rego Casasnovas from comment #61) > Comment on attachment 331356 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331356&action=review > > The patch LGTM. > > Just a question about border-styles-vertical-lr.html test, for the new > expected files > I see color changes in the borders, is that expected? Yes, it's expected because those colors are rendered differently now in gtk+. Since the PNGs are not used unless there is a diff in the expected.txt files, we haven't noticed such color differences in the PNGs. Created attachment 332756 [details]
Patch
Comment on attachment 332756 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332756&action=review r=me > Source/WebCore/rendering/RenderBlockFlow.cpp:3064 > + if (style.isFlippedLinesWritingMode()) > + lastBaseline = logicalHeight() - lastRootBox()->logicalBottom() + style.fontMetrics().ascent(lastRootBox()->baselineType()); > + else > + lastBaseline = lastRootBox()->logicalTop() + style.fontMetrics().ascent(lastRootBox()->baselineType()); Nit: Maybe we can write this like: lastBaseline = style.isFlippedLinesWritingMode() ? logicalHeight() - lastRootBox()->logicalBottom() : lastRootBox()->logicalTop(); lastBaseline += style.fontMetrics().ascent(lastRootBox()->baselineType()); Created attachment 332759 [details]
Patch
Created attachment 332769 [details]
Patch
Comment on attachment 332769 [details] Patch Attachment 332769 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6295278 New failing tests: fast/writing-mode/text-orientation-basic.html Created attachment 332790 [details]
Archive of layout-test-results from ews202 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 332794 [details]
Patch
Comment on attachment 332794 [details] Patch Clearing flags on attachment: 332794 Committed r227947: <https://trac.webkit.org/changeset/227947> All reviewed patches have been landed. Closing bug. Test that were introduced with this patch: fast/inline-block/baseline-vertical-01.html fast/inline-block/baseline-vertical-02.html fast/inline-block/baseline-vertical-03.html fast/inline-block/baseline-vertical-04.html are showing up as ImageOnlyFailures on iOS simulator. Looks like you intended to mark them as failing for iOS Simulator. Judging by the EWS output, it should have been marked [ ImageOnlyFailure ] instead of just failure. Went ahead and fixed it for you in: https://trac.webkit.org/changeset/227975/webkit |
Created attachment 305586 [details] Test case to reproduce the issue A vertical-lr block with inline-block children does not compute correctly their baseline so they can be alignment properly. This issue is not present when using other writing-modes Attached a file with several test cases to reproduce the issue using different writing-modes and direction.