Bug 47706 - Add "in select" insertion mode to parser.
Summary: Add "in select" insertion mode to parser.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 46676
  Show dependency treegraph
 
Reported: 2010-10-14 18:19 PDT by James Simonsen
Modified: 2010-10-19 17:57 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.12 KB, patch)
2010-10-14 18:24 PDT, James Simonsen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Simonsen 2010-10-14 18:19:50 PDT
Add "in select" insertion mode to parser.
Comment 1 James Simonsen 2010-10-14 18:24:42 PDT
Created attachment 70815 [details]
Patch
Comment 2 James Simonsen 2010-10-14 18:25:51 PDT
I couldn't figure out how to contrive a test case to exercise this. If you know how, please let me know.
Comment 3 Adam Barth 2010-10-15 00:42:08 PDT
Comment on attachment 70815 [details]
Patch

From the ASSERT, it looks like you'll need to parse a fragment.  Maybe try setting the innerHTML of an <option> element to "</select>" ?

<select>
 <option>xxx</option>
</select>

You might need to mix in some optgroup elements or something to see the difference between table scope and select scope.

r- for no test.  Tests for these sorts of changes are essential.
Comment 4 James Simonsen 2010-10-18 14:40:29 PDT
I don't think it's possible to test.

The innerHTML of <option> doesn't work because that's processed in "in body" mode. Only <select> starts "in select" mode.

From "in select" mode, there doesn't seem to be any way to insert an element that would change the scope, but not the mode.
Comment 5 Adam Barth 2010-10-19 13:07:29 PDT
Comment on attachment 70815 [details]
Patch

We looked at this change in detail.  We don't believe it's observable because it's not possible to have a select element in the stack of open elements when in fragment mode.
Comment 6 WebKit Commit Bot 2010-10-19 13:53:47 PDT
Comment on attachment 70815 [details]
Patch

Rejecting patch 70815 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 70815]" exit_code: 2
Last 500 characters of output:
atform/win/Skipped
	M	LayoutTests/ChangeLog
r70080 = 6b74d470f67a7767229ffea11b1571769f0f5ce5 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
error: Untracked working tree file 'LayoutTests/fast/dom/nodesFromRect-links-and-text-expected.txt' would be overwritten by merge.
could not detach HEAD
rebase refs/remotes/trunk: command returned error: 1

Died at WebKitTools/Scripts/update-webkit line 129.

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4558003
Comment 7 Eric Seidel (no email) 2010-10-19 14:03:22 PDT
Eek.  I'lll see if I can figure out which bot has the screwy checkout.
Comment 8 Adam Barth 2010-10-19 14:24:07 PDT
(In reply to comment #7)
> Eek.  I'lll see if I can figure out which bot has the screwy checkout.

Is there a command line argument we can pass to run-webkit-tests to tell it not to generated results for new tests?
Comment 9 Eric Seidel (no email) 2010-10-19 14:28:36 PDT
  --[no-]new-test-results         Generate results for new tests
Comment 10 Adam Barth 2010-10-19 14:49:14 PDT
Comment on attachment 70815 [details]
Patch

Adam's machine's checkout was wedged (eseidel)
Comment 11 WebKit Commit Bot 2010-10-19 15:16:29 PDT
Comment on attachment 70815 [details]
Patch

Clearing flags on attachment: 70815

Committed r70094: <http://trac.webkit.org/changeset/70094>
Comment 12 WebKit Commit Bot 2010-10-19 15:16:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Adam Barth 2010-10-19 17:57:36 PDT
(In reply to comment #9)
>   --[no-]new-test-results         Generate results for new tests

We should definitely add that.