Created attachment 234560 [details] Path proposal I would love to have more tests converted in a go, but the ones related to text alignment need to be converted separately (see bug 129313 and bug 134724)

Comment on attachment 234560 [details] Path proposal View in context: https://bugs.webkit.org/attachment.cgi?id=234560&action=review > LayoutTests/fast/css/text-overflow-ellipsis-expected.html:18 > +.forcertl { > + direction: rtl; > + unicode-bidi: bidi-override; > +} I don't think we should be relying on UBA in the expected result for this test given where ellipses will be drawn will be affected by UBA itself. Reorder the content manually instead. > LayoutTests/fast/css/text-overflow-ellipsis-strict-expected.html:18 > + direction: rtl; > + unicode-bidi: bidi-override; Ditto.

Comment on attachment 234560 [details] Path proposal Attachment 234560 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4782096186867712 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html fast/dom/HTMLImageElement/sizes/image-sizes-js-change.html

Created attachment 234562 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Created attachment 234563 [details] Path proposal (In reply to comment #2) > (From update of attachment 234560 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234560&action=review > > > LayoutTests/fast/css/text-overflow-ellipsis-expected.html:18 > > +.forcertl { > > + direction: rtl; > > + unicode-bidi: bidi-override; > > +} > > I don't think we should be relying on UBA in the expected result for this test given where ellipses will be drawn will be affected by UBA itself. > Reorder the content manually instead. > Good point. Attaching now a new patch following that suggestion. > > LayoutTests/fast/css/text-overflow-ellipsis-strict-expected.html:18 > > + direction: rtl; > > + unicode-bidi: bidi-override; > > Ditto.

Comment on attachment 234563 [details] Path proposal Attachment 234563 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4560418697641984 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 234575 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Comment on attachment 234563 [details] Path proposal Attachment 234563 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5382742799810560 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 234579 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Comment on attachment 234563 [details] Path proposal Attachment 234563 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5759239632977920 New failing tests: media/W3C/video/src/src_reflects_attribute_not_source_elements.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html fast/css/text-overflow-ellipsis-strict.html

Created attachment 234582 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Comment on attachment 234563 [details] Path proposal Attachment 234563 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5413391686434816 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 234590 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

(In reply to comment #12) > (From update of attachment 234563 [details]) > Attachment 234563 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/5413391686434816 > > New failing tests: > fast/css/text-overflow-ellipsis-strict.html > fast/css/text-overflow-ellipsis-bidi.html > fast/css/text-overflow-ellipsis.html For some reason that I can't explain, the actual and expected result provided by the ref test do not match in the Mac because of one additional character being printed in the Mac before the ellipsis. I've tried to do a WebKit build in a Mac I have over here, but then faced some other problems with Xcode that I'm still trying to decipher and couldn't advance much either. Any idea?

Comment on attachment 234563 [details] Path proposal Clearing the r? since it's clear this patch can't get in

Created attachment 235220 [details] Patch proposal (trying simply with a monospace font-family) This small change seemed to help in the local experiments I did. Let's see what the EWS think about it...

Comment on attachment 235220 [details] Patch proposal (trying simply with a monospace font-family) Attachment 235220 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5428177178460160 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 235221 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Comment on attachment 235220 [details] Patch proposal (trying simply with a monospace font-family) Attachment 235220 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4712264749809664 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 235223 [details] Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Comment on attachment 235220 [details] Patch proposal (trying simply with a monospace font-family) Attachment 235220 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5741758746984448 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis-bidi.html fast/css/text-overflow-ellipsis.html

Created attachment 235228 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5

It seems that part of the reason why these tests are failing here only in the mac is because of the calls to drawSkipInkUnderline() functions, which are not being called in WebKitGTK+ (because of the ENABLE(CSS3_TEXT_DECORATION_SKIP_INK) guard). When that function is called, translateIntersectionPointsToSkipInkBoundaries() is called too where it seems that "way too many" dashes (and the gaps in the middle) are being returned as the result DashArray. Reason for that seems to be that is not taking into consideration the fact that the whole text might be ellipsized, and therefore adding tuples of coordinates definining those gaps which are out of the range defined by the visible part of the text is wrong, causing the underline to show up shorter than what it actually is if there's a single character later on the line requiring a "ink skip". The following experiment "fixes" some cases of this weird behaviour, even though it does not seem correct (we would probably need to properly consider text direction and things like that): --- a/Source/WebCore/rendering/InlineTextBox.cpp +++ b/Source/WebCore/rendering/InlineTextBox.cpp @@ -80,8 +80,11 @@ static DashArray translateIntersectionPointsToSkipInkBoundaries(const DashArray& // Step 1: Make pairs so we can sort based on range starting-point. We dilate the ranges in this step as well. Vector<std::pair<float, float>> tuples; - for (auto i = intersections.begin(); i != intersections.end(); i++, i++) + for (auto i = intersections.begin(); i != intersections.end(); i++, i++) { + if (*i <= -totalWidth || *i >= totalWidth) + continue; tuples.append(std::make_pair(*i - dilationAmount, *(i + 1) + dilationAmount)); + } Unfortunately, I don't have much time anymore this week and I'll be away for a while after it, so I can't keep devoting time to this issue (which happens to show up in the Mac only, btw), so I'm posting this here in the hope that someone with a better understanding of this part can take a look and see whether it makes sense or not. PS: drawLineForText() works perfectly fine, the problem is with drawSkipInkUnderline() because translateIntersectionPointsToSkipInkBoundaries() returns gaps not belonging to the visible (not wrapped) test, which causes that weird effect.