Bug 51379

Summary:

Convert <keygen> option elements to a shadow DOM

Product:

WebKit

Reporter:

Dominic Cooney <dominicc>

Component:

DOM

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, dglazkov, eric, loislo, morrita, mrobinson, ossy

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

52557

Bug Blocks:

48698, 53030

Attachments:

Description	Flags
WIP--adds IDL for keygen element	none
WIP—moves select into shadow DOM	none
Patch.	dglazkov: review+
Unskips html5lib/runner.html for Qt and GTK	eric: review+, commit-queue: commit-queue-

Dominic Cooney

Reported 2010-12-20 22:18:56 PST

<keygen> needs a shadow DOM to hold its <option> elements. Right now they're just added to the DOM directly, which causes us to skip the html5 parsing tests on all platforms other than mac (since the current expected DOM is the one which appears for Mac.) This came up in the review for <https://bugs.webkit.org/show_bug.cgi?id=44907>

Attachments
WIP--adds IDL for keygen element (7.80 KB, patch) 2011-01-13 01:58 PST, Dominic Cooney	no flags	Details Formatted Diff Diff
WIP—moves select into shadow DOM (6.52 KB, patch) 2011-02-03 01:33 PST, Dominic Cooney	no flags	Details Formatted Diff Diff
Patch. (15.79 KB, patch) 2011-02-04 01:15 PST, Dominic Cooney	dglazkov: review+	Details Formatted Diff Diff
Unskips html5lib/runner.html for Qt and GTK (17.15 KB, patch) 2011-02-06 01:36 PST, Dominic Cooney	eric: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2010-12-20 22:21:29 PST

I'm certain we already have a old bug on this. :)

Eric Seidel (no email)

Comment 2 2010-12-20 22:22:18 PST

Hmm.. I guess it was only ever discussed in https://bugs.webkit.org/show_bug.cgi?id=42776. Maybe a bug about this specific issue was never filed.

Adam Barth

Comment 3 2010-12-20 23:37:34 PST

Thanks dominicc. I've been hoping someone would fix this bug. :)

Dominic Cooney

Comment 4 2011-01-13 01:58:35 PST

Created attachment 78788 [details] WIP--adds IDL for keygen element

Dominic Cooney

Comment 5 2011-02-03 01:33:37 PST

Created attachment 81042 [details] WIP—moves select into shadow DOM WIP--moves select into shadow DOM but does not forward form protocol yet

Dominic Cooney

Comment 6 2011-02-04 01:15:29 PST

Created attachment 81197 [details] Patch. Chromium expectations need to be updated.

Dimitri Glazkov (Google)

Comment 7 2011-02-04 07:45:55 PST

Comment on attachment 81197 [details] Patch. This looks great!

Adam Barth

Comment 8 2011-02-04 09:49:33 PST

So exciting.

Eric Seidel (no email)

Comment 9 2011-02-04 13:27:29 PST

We shoudl also now remove the parser test from the windows/gtk/qt skipped lists. :)

Dominic Cooney

Comment 10 2011-02-06 01:36:02 PST

Created attachment 81395 [details] Unskips html5lib/runner.html for Qt and GTK

Eric Seidel (no email)

Comment 11 2011-02-06 02:17:12 PST

Comment on attachment 81395 [details] Unskips html5lib/runner.html for Qt and GTK LGTM.

WebKit Commit Bot

Comment 12 2011-02-06 02:38:42 PST

Comment on attachment 81395 [details] Unskips html5lib/runner.html for Qt and GTK Rejecting attachment 81395 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'land-a..." exit_code: 1 Last 500 characters of output: ommit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run step(tool, options).run(state) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 57, in run if self._has_valid_reviewer(changelog_entry): File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 42, in _has_valid_reviewer if changelog_entry.reviewer(): AttributeError: 'NoneType' object has no attribute 'reviewer' Full output: http://queues.webkit.org/results/7700543

Eric Seidel (no email)

Comment 13 2011-02-06 10:15:21 PST

Comment on attachment 81395 [details] Unskips html5lib/runner.html for Qt and GTK I'm not sure why the cq did that. Lets try again.

WebKit Commit Bot

Comment 14 2011-02-06 11:09:35 PST

Comment on attachment 81395 [details] Unskips html5lib/runner.html for Qt and GTK Rejecting attachment 81395 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'land-a..." exit_code: 1 Last 500 characters of output: ommit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run step(tool, options).run(state) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 57, in run if self._has_valid_reviewer(changelog_entry): File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 42, in _has_valid_reviewer if changelog_entry.reviewer(): AttributeError: 'NoneType' object has no attribute 'reviewer' Full output: http://queues.webkit.org/results/7707156

Eric Seidel (no email)

Comment 15 2011-02-06 11:17:51 PST

Ah, I see. That would suggest you're missing a ChangeLog. But I thought we had a better message for that.

Dominic Cooney

Comment 16 2011-02-06 15:51:34 PST

My fault. I went from work to home and snarfed the patch with webkit-patch apply. It must have helpfully filled in dglazkov since the previous patch was r+.

Hajime Morrita

Comment 17 2011-02-06 17:40:37 PST

Committed r77781: <http://trac.webkit.org/changeset/77781>

Ilya Tikhonovsky

Comment 18 2011-02-09 04:43:17 PST

new expectations were added for chromium http://trac.webkit.org/changeset/78046 http://trac.webkit.org/changeset/78037

Eric Seidel (no email)

Comment 19 2011-02-09 05:00:31 PST

Chromium shouldn't need separate expectations anymore. Do you know why it still has its own -expected.txt file?

Ilya Tikhonovsky

Comment 20 2011-02-09 05:09:09 PST

(In reply to comment #19) > Chromium shouldn't need separate expectations anymore. Do you know why it still has its own -expected.txt file? There is a difference in SyntaxError text for html5lib/runner.html. loislo@loislo0-g:/var/sdb1/loislo/chromium/src/third_party/WebKit$ diff LayoutTests/html5lib/runner-expected.txt LayoutTests/platform/chromium/html5lib/runner-expected.txt 1,2c1,2 < CONSOLE MESSAGE: line 1: SyntaxError: Parse error < CONSOLE MESSAGE: line 1: SyntaxError: Parse error --- > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < 6,9c6,9 < CONSOLE MESSAGE: line 1: SyntaxError: Parse error < CONSOLE MESSAGE: line 1: SyntaxError: Parse error < CONSOLE MESSAGE: line 1: SyntaxError: Parse error < CONSOLE MESSAGE: line 1: SyntaxError: Parse error --- > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < > CONSOLE MESSAGE: line 1: Uncaught SyntaxError: Unexpected token < and 2 pixel difference of vertical size for fast/invalid/residual-style-expected.txt.

Adam Barth

Comment 21 2011-02-09 11:13:24 PST

(In reply to comment #19) > Chromium shouldn't need separate expectations anymore. Do you know why it still has its own -expected.txt file? It's because V8's error messages don't match those of JSC. We've tried getting the V8 team to change, but they refuse. I'd really like to know how much engineering time that costs us.

Note You need to log in before you can comment on or make changes to this bug.