Bug 51379 - Convert <keygen> option elements to a shadow DOM
Summary: Convert <keygen> option elements to a shadow DOM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 52557
Blocks: 48698 53030
  Show dependency treegraph
 
Reported: 2010-12-20 22:18 PST by Dominic Cooney
Modified: 2011-02-09 11:13 PST (History)
8 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 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>
Comment 1 Eric Seidel (no email) 2010-12-20 22:21:29 PST
I'm certain we already have a old bug on this.  :)
Comment 2 Eric Seidel (no email) 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.
Comment 3 Adam Barth 2010-12-20 23:37:34 PST
Thanks dominicc.  I've been hoping someone would fix this bug.  :)
Comment 4 Dominic Cooney 2011-01-13 01:58:35 PST
Created attachment 78788 [details]
WIP--adds IDL for keygen element
Comment 5 Dominic Cooney 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
Comment 6 Dominic Cooney 2011-02-04 01:15:29 PST
Created attachment 81197 [details]
Patch.

Chromium expectations need to be updated.
Comment 7 Dimitri Glazkov (Google) 2011-02-04 07:45:55 PST
Comment on attachment 81197 [details]
Patch.

This looks great!
Comment 8 Adam Barth 2011-02-04 09:49:33 PST
So exciting.
Comment 9 Eric Seidel (no email) 2011-02-04 13:27:29 PST
We shoudl also now remove the parser test from the windows/gtk/qt skipped lists. :)
Comment 10 Dominic Cooney 2011-02-06 01:36:02 PST
Created attachment 81395 [details]
Unskips html5lib/runner.html for Qt and GTK
Comment 11 Eric Seidel (no email) 2011-02-06 02:17:12 PST
Comment on attachment 81395 [details]
Unskips html5lib/runner.html for Qt and GTK

LGTM.
Comment 12 WebKit Commit Bot 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
Comment 13 Eric Seidel (no email) 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.
Comment 14 WebKit Commit Bot 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
Comment 15 Eric Seidel (no email) 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.
Comment 16 Dominic Cooney 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+.
Comment 17 Hajime Morrita 2011-02-06 17:40:37 PST
Committed r77781: <http://trac.webkit.org/changeset/77781>
Comment 18 Ilya Tikhonovsky 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
Comment 19 Eric Seidel (no email) 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?
Comment 20 Ilya Tikhonovsky 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.
Comment 21 Adam Barth 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.