RESOLVED FIXED 180818
Line clamp implementation
https://bugs.webkit.org/show_bug.cgi?id=180818
Summary Line clamp implementation
Dave Hyatt
Reported 2017-12-14 10:24:12 PST
Line clamp parsing
Attachments
Patch (12.97 KB, patch)
2017-12-14 10:26 PST, Dave Hyatt
no flags
Patch (12.77 KB, patch)
2017-12-14 10:33 PST, Dave Hyatt
no flags
Patch (14.25 KB, patch)
2017-12-14 10:51 PST, Dave Hyatt
no flags
Patch (71.48 KB, patch)
2018-01-16 12:41 PST, Dave Hyatt
ews-watchlist: commit-queue-
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.35 MB, application/zip)
2018-01-16 14:19 PST, EWS Watchlist
no flags
Patch (71.47 KB, patch)
2018-01-17 09:08 PST, Dave Hyatt
ews-watchlist: commit-queue-
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.35 MB, application/zip)
2018-01-17 10:49 PST, EWS Watchlist
no flags
Patch (79.71 KB, patch)
2018-01-18 08:41 PST, Dave Hyatt
no flags
Patch (82.28 KB, patch)
2018-01-18 08:52 PST, Dave Hyatt
no flags
Patch (91.46 KB, patch)
2018-01-24 14:25 PST, Dave Hyatt
dino: review+
Dave Hyatt
Comment 1 2017-12-14 10:26:55 PST
EWS Watchlist
Comment 2 2017-12-14 10:29:04 PST
Attachment 329361 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/style/LineClampValue.h:66: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dave Hyatt
Comment 3 2017-12-14 10:33:22 PST
Dave Hyatt
Comment 4 2017-12-14 10:51:51 PST
Alex Christensen
Comment 5 2017-12-14 12:10:08 PST
Comment on attachment 329364 [details] Patch It looks like you forgot to add the actual test. This patch only contains expectations.
Dave Hyatt
Comment 6 2018-01-16 12:41:22 PST
EWS Watchlist
Comment 7 2018-01-16 12:43:41 PST
Attachment 331414 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/RenderLinesClampFlow.cpp:27: You should add a blank line after implementation file's own header. [build/include_order] [4] ERROR: Source/WebCore/rendering/RenderLinesClampSet.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:60: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:106: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:110: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:129: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:139: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/rendering/RenderLinesClampSet.cpp:150: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 8 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 8 2018-01-16 14:19:55 PST
Comment on attachment 331414 [details] Patch Attachment 331414 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6097390 New failing tests: fast/overflow/line-clamp.html
EWS Watchlist
Comment 9 2018-01-16 14:19:57 PST
Created attachment 331429 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Dave Hyatt
Comment 10 2018-01-17 09:08:58 PST
EWS Watchlist
Comment 11 2018-01-17 10:49:10 PST
Comment on attachment 331500 [details] Patch Attachment 331500 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6107002 New failing tests: fast/overflow/line-clamp.html
EWS Watchlist
Comment 12 2018-01-17 10:49:12 PST
Created attachment 331518 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Dave Hyatt
Comment 13 2018-01-18 08:41:23 PST
Dave Hyatt
Comment 14 2018-01-18 08:52:23 PST
Dave Hyatt
Comment 15 2018-01-24 14:25:02 PST
Dean Jackson
Comment 16 2018-01-24 14:36:53 PST
Comment on attachment 332200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332200&action=review How will Mail toggle the setting? Is it already exposed as a WebPreference? > Source/WebCore/rendering/RenderBlockFlow.cpp:3238 > + // FIXME: Orthogonal writing modes don't work here, but it's not even clear how they should be behave anyway. Nit: should be behave > Source/WebCore/rendering/RenderLinesClampFlow.h:39 > + void layout() override; Did you need to do this, since you just call RenderMultiColumnFlow::layout()? > Source/WebCore/rendering/RenderLinesClampSet.cpp:188 > + Nit: remove blank line
Dave Hyatt
Comment 17 2018-01-24 14:53:15 PST
Landed in r227577.
Note You need to log in before you can comment on or make changes to this bug.