Bug 171322 - Align colspan/rowspan limits with the latest HTML specification
Summary: Align colspan/rowspan limits with the latest HTML specification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified All
: P2 Normal
Assignee: Chris Dumez
URL: https://html.spec.whatwg.org/#dom-tdt...
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2017-04-26 07:38 PDT by Simon Pieters (:zcorpan)
Modified: 2017-04-27 18:54 PDT (History)
12 users (show)

See Also:


Attachments
WIP Patch (31.26 KB, patch)
2017-04-26 14:10 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (32.85 KB, patch)
2017-04-26 14:40 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (32.80 KB, patch)
2017-04-26 14:55 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (34.02 KB, patch)
2017-04-26 14:58 PDT, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1.02 MB, application/zip)
2017-04-26 15:50 PDT, Build Bot
no flags Details
Patch (42.14 KB, patch)
2017-04-26 15:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (863.73 KB, application/zip)
2017-04-26 17:41 PDT, Build Bot
no flags Details
Patch (51.04 KB, patch)
2017-04-26 19:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (7.98 MB, application/zip)
2017-04-26 21:37 PDT, Build Bot
no flags Details
Patch (429.75 KB, patch)
2017-04-26 22:41 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (47.87 KB, patch)
2017-04-26 22:46 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (52.78 KB, patch)
2017-04-27 11:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (62.02 KB, patch)
2017-04-27 16:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (62.12 KB, patch)
2017-04-27 17:01 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters (:zcorpan) 2017-04-26 07:38:57 PDT
HTML spec change:
https://github.com/whatwg/html/pull/1993

Tests:
https://github.com/w3c/web-platform-tests/pull/4115
Comment 1 Chris Dumez 2017-04-26 14:10:56 PDT
Created attachment 308282 [details]
WIP Patch
Comment 2 Chris Dumez 2017-04-26 14:11:30 PDT
Comment on attachment 308282 [details]
WIP Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308282&action=review

> LayoutTests/imported/w3c/web-platform-tests/html/dom/reflection-tabular-expected.txt:4308
> +FAIL th.colSpan: setAttribute() to 2147483648 assert_equals: IDL get expected 1000 but got 1

Still investigating those failures... not sure if the test is wrong or my implementation yet.
Comment 3 Chris Dumez 2017-04-26 14:40:12 PDT
Created attachment 308284 [details]
WIP Patch
Comment 4 Chris Dumez 2017-04-26 14:55:51 PDT
Created attachment 308288 [details]
WIP Patch
Comment 5 Chris Dumez 2017-04-26 14:58:19 PDT
Created attachment 308290 [details]
WIP Patch
Comment 6 Build Bot 2017-04-26 15:50:14 PDT
Comment on attachment 308290 [details]
WIP Patch

Attachment 308290 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3613480

New failing tests:
fast/table/giantRowspan.html
fast/table/giantRowspan2.html
Comment 7 Build Bot 2017-04-26 15:50:15 PDT
Created attachment 308294 [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 8 Chris Dumez 2017-04-26 15:56:14 PDT
Created attachment 308297 [details]
Patch
Comment 9 Build Bot 2017-04-26 17:41:16 PDT
Comment on attachment 308297 [details]
Patch

Attachment 308297 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3614647

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
fast/table/giantRowspan.html
Comment 10 Build Bot 2017-04-26 17:41:18 PDT
Created attachment 308313 [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.11.6
Comment 11 Chris Dumez 2017-04-26 19:29:47 PDT
Created attachment 308323 [details]
Patch
Comment 12 Build Bot 2017-04-26 21:36:57 PDT
Comment on attachment 308323 [details]
Patch

Attachment 308323 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3615949

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
Comment 13 Build Bot 2017-04-26 21:37:03 PDT
Created attachment 308334 [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.11.6
Comment 14 Chris Dumez 2017-04-26 22:41:54 PDT
Created attachment 308339 [details]
Patch
Comment 15 Chris Dumez 2017-04-26 22:46:15 PDT
Created attachment 308340 [details]
Patch
Comment 16 Chris Dumez 2017-04-27 11:59:02 PDT
Created attachment 308421 [details]
Patch
Comment 17 Chris Dumez 2017-04-27 15:28:25 PDT
Comment on attachment 308421 [details]
Patch

Patch is ready for review.
Comment 18 Darin Adler 2017-04-27 15:43:11 PDT
Comment on attachment 308421 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308421&action=review

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:157
> +static HTMLIntegerParsingError parseHTMLIntegerInternal(const CharacterType* position, const CharacterType* end, int& result)

Should this use std::expected instead?
Comment 19 Alex Christensen 2017-04-27 15:43:46 PDT
I was commenting the same thing.  3x
Comment 20 Chris Dumez 2017-04-27 16:35:12 PDT
Will re upload a patch using WTF::Expected. It does look much better.
Comment 21 Chris Dumez 2017-04-27 16:57:36 PDT
Created attachment 308481 [details]
Patch
Comment 22 Chris Dumez 2017-04-27 17:01:48 PDT
Created attachment 308482 [details]
Patch
Comment 23 WebKit Commit Bot 2017-04-27 18:53:59 PDT
Comment on attachment 308482 [details]
Patch

Clearing flags on attachment: 308482

Committed r215914: <http://trac.webkit.org/changeset/215914>
Comment 24 WebKit Commit Bot 2017-04-27 18:54:02 PDT
All reviewed patches have been landed.  Closing bug.