Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(372)

Issue 1955863002: Make menuitem non-void element.

Created:
4 years, 7 months ago by pals
Modified:
4 years, 6 months ago
Reviewers:
tkent
CC:
chromium-reviews, blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org, kinuko+watch
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make menuitem non-void element. Allow menuitem to have endtag and menuitem element's end tag can be omitted if the menuitem element is immediately followed by a menuitem, hr, or menu element, or if there is no more content in the parent element. Specifications: http://www.whatwg.org/specs/web-apps/current-work/#the-menuitem-element BUG=87553

Patch Set 1 #

Patch Set 2 : NeedsManualRebaseline #

Patch Set 3 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -11 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/parser/parse-menuitem.html View 1 chunk +7 lines, -5 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/parser/parse-menuitem-expected.txt View 1 chunk +6 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLElement.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp View 3 chunks +25 lines, -2 lines 0 comments Download

Messages

Total messages: 37 (14 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1955863002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1955863002/1
4 years, 7 months ago (2016-05-06 12:54:03 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/224624)
4 years, 7 months ago (2016-05-06 13:56:40 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1955863002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1955863002/20001
4 years, 7 months ago (2016-05-09 04:48:29 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/2244) mac_chromium_gn_rel on ...
4 years, 7 months ago (2016-05-09 04:49:55 UTC) #9
pals
imported/web-platform-tests/html/syntax/parsing/html5lib_tests25.html expected: #document\n| <!DOCTYPE html>\n| <html>\n| <head>\n| <body>\n| <menuitem>\n| \"A\" actual : #document\n| <!DOCTYPE html>\n| ...
4 years, 7 months ago (2016-05-09 04:50:34 UTC) #10
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1955863002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1955863002/40001
4 years, 7 months ago (2016-05-09 05:09:38 UTC) #12
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: ios-device-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/builds/2267) ios-simulator on ...
4 years, 7 months ago (2016-05-09 05:11:29 UTC) #14
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1955863002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1955863002/40001
4 years, 7 months ago (2016-05-10 04:24:12 UTC) #17
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/2841) ios-device-gn on ...
4 years, 7 months ago (2016-05-10 04:26:22 UTC) #19
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1955863002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1955863002/60001
4 years, 7 months ago (2016-05-10 06:08:18 UTC) #22
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-10 09:19:41 UTC) #24
pals
Please take a look.
4 years, 7 months ago (2016-05-10 11:11:26 UTC) #25
pals
imported/web-platform-tests/html/syntax/parsing/html5lib_tests25.html expected: #document\n| <!DOCTYPE html>\n| <html>\n| <head>\n| <body>\n| <menuitem>\n| \"A\" actual : #document\n| <!DOCTYPE html>\n| ...
4 years, 7 months ago (2016-05-10 11:13:02 UTC) #26
tkent
Do you know this change won't fix crbug.com/412945, and won't unblock shipping the feature?
4 years, 7 months ago (2016-05-11 00:47:50 UTC) #27
pals
On 2016/05/11 00:47:50, tkent wrote: > Do you know this change won't fix crbug.com/412945, and ...
4 years, 7 months ago (2016-05-12 03:10:03 UTC) #28
tkent
I think we can ship the feature if - Fix crbug.com/412945, or - Prove that ...
4 years, 7 months ago (2016-05-12 03:29:57 UTC) #29
pals
On 2016/05/12 03:29:57, tkent wrote: > I think we can ship the feature if > ...
4 years, 7 months ago (2016-05-12 06:31:45 UTC) #30
tkent
> The current specification is fixing crbug.com/412945. The specification says "Allow menuitem to have endtag ...
4 years, 7 months ago (2016-05-12 07:58:58 UTC) #31
tkent
> The current specification is fixing crbug.com/412945. The specification says "Allow menuitem to have endtag ...
4 years, 7 months ago (2016-05-12 07:59:41 UTC) #32
pals
On 2016/05/12 07:59:41, tkent wrote: > > The current specification is fixing crbug.com/412945. The specification ...
4 years, 7 months ago (2016-05-12 08:50:27 UTC) #33
pals
On 2016/05/12 08:50:27, pals wrote: > On 2016/05/12 07:59:41, tkent wrote: > > > The ...
4 years, 7 months ago (2016-05-12 08:52:09 UTC) #34
pals
Checked with the spec authors. As you said, menuitem cannot have children. So, this spec ...
4 years, 6 months ago (2016-06-15 06:38:12 UTC) #36
tkent
4 years, 6 months ago (2016-06-15 06:48:36 UTC) #37
On 2016/06/15 at 06:38:12, sanjoy.pal wrote:
> Checked with the spec authors. As you said, menuitem cannot have children. So,
this spec change does not fix the apple site issue.
> As this patch set make menuitem more spec compliant, can you please review
patch set 3?

You should work on changing the specification so that it is compatible with
Firefox and the Apple site.
Making Blink spec-compliant now helps nothing.

Powered by Google App Engine
This is Rietveld 408576698