New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 16 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 87553



Sign in to add a comment

<menuitem> parsing is no web-compatible

Project Member Reported by dsinclair@chromium.org, Sep 10 2014

Issue description

If you open the below link in M39 (39.0.2145.4 (Official Build)) everything appears to sit on top of itself. The page renders correctly in M37. I don't have M38 to test with locally.

The scrolling is also very busted.

https://developer.apple.com/library/prerelease/ios/documentation/HealthKit/Reference/HealthKit_Constants/index.html#//apple_ref/c/econst/HKBodyTemperatureSensorLocationRectum
 
Screenshot from 2014-09-10 17:01:50.png
192 KB View Download
This does not reproduce on M39 mac Retina. It does reproduce on Linux and Mac non-retina.
Cc: a1.go...@samsung.com
The website has the following:

<meta name="viewport" content="width=device-width, maximum-scale=1.0">

I wonder if that's not the trigger. +Antonio as he knows this code well.
Labels: Needs-Feedback
dsinclair@,I working on Windows7/Linux(Ubuntu 12.04)/MAC (OSX 10.9.4)-MACBook Air and i am unable to reproduce the above issue as per screen shot with reported version men ,M38 (38.0.2125.58 (Official Build 291101) beta and with Latest canary :39.0.2152.0(M39).

When comes to Scrolling i could find issue with MAC  when "On this Page" drop down is clicked and by clicking down arrow till last item and further  then we can find scroll bar appears and disappears but it doesn't stay there , its working fine with windows and linux.

More over the link  which leads to the page i randomly moved to all the links inside the page but i could not find any such rendering issues or issue shown is screen shot , when i observed screen shot it hows the text "filter list of symbols" in search box given but when  went with the link i could find the text "Search IOS developer Library" ,i tried navigating to different links but i could not find the searchbox with text which is shown in the screen shot ,so i have a doubt that weather i am in right page or not,Please let me know if i am missing any step.
 
Labels: -Needs-Feedback
So, apparently you need to enable 'Experimental Web Platform Features' in about://flags and then everything blows up and you see the above rendering.
Labels: -Type-Bug -Needs-Bisect Type-Bug-Regression
Owner: mkwst@chromium.org
Status: Assigned
Able to reproduce the issue in 39.0.2155.0 canary and  38.0.2125.54 beta. Issue not seen in M37 stable.

Broken in M38 beta. 
38.0.2121.3 - Good build
38.0.2122.2 - Bad build

Narrow bisect CL is:
https://chromium.googlesource.com/chromium/src/+log/fc6035eeae866d4afb10242159fbd8dc2847dce3..cb618bea7e766a084a8b9a431425330b9f0c155f

Narrow blink bisect CL is:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?url=/trunk&range=180058%3A180034

Suspecting 180052, 180040. mkwst@, would you please take a look at this issue. Please assign it to concerned Dev if this is not your change.

Comment 6 by mkwst@chromium.org, Sep 12 2014

Owner: ----
This is almost certainly not 180040. That CL adds an (as-yet unused) platform interface to Blink. There are a number of CSS-related patches in the range you've specified. I'd take a closer look at those, as they're more likely to have something to do with rendering behavior.
Cc: tkent@chromium.org
Owner: sanjoy....@samsung.com
I think the CSS properties are not related. The page uses <menuitem> and we have a change in the range -> https://codereview.chromium.org/443373002
Yes, this is broken because of this change https://codereview.chromium.org/443373002/diff/120001/Source/core/html/parser/HTMLTreeBuilder.cpp.
But as per the specification here http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#the-menuitem-element
<menuitem> should not have any end tag, its a self closing tag.
The page packs some html elements in between <menuitem></menuitem>, which are ignored. So, its a problem with the page source.

Comment 9 by tkent@chromium.org, Sep 15 2014

Labels: Cr-Blink-HTML Cr-Blink-HTML-Menu
Status: WontFix
Yeah, this is a web-site bug.

 Issue 429538  has been merged into this issue.
 Issue 434240  has been merged into this issue.

Comment 12 by pdr@chromium.org, Nov 18 2014

Why doesn't firefox have the same issue we're seeing here?
Given that chrome apps, apple websites and firefox all agree we should just
change the spec to match firefox instead of fighting this up hill.

Comment 15 by pdr@chromium.org, Nov 18 2014

I agree with esprehn, we should revert this change and change the spec instead.

Comment 16 by pdr@chromium.org, Nov 19 2014

Internally I found more folks who weren't able to use menuitem correctly (go/csmenuitem, sorry but this is only available to Google employees). Is this really worth following the spec so strictly here? What's the cost of allowing non-self-closing tags vs the benefit of not breaking these existing uses?

Comment 17 by tkent@chromium.org, Nov 19 2014

Labels: -Cr-Blink-Rendering
I'm not sure if there are many broken sites.  We should collect data.  e.g. UseCounter for <menuitem> and <menuitem> with child nodes.

Comment 18 by pdr@chromium.org, Nov 21 2014

Status: Untriaged
I did a search and found a few more sites but no major ones, other than Apple. I also found that menuitem is not particularly popular, other than Apple. Following the spec seems less important than preventing our users from reading the apple documentation.

@sanjoy.pal@samsung.com, what is the downside to not following the spec here? I think apple.com is important enough that we should find a way to contact apple, or revert this change.
I think there will not be any issue if we deviate from spec here as we are using "label" of <menuitem> to populate the context menu and ignoring anything between <menuitem></menuitem>. If there is consensus about deviating from spec, I can upload a patch for review.  

Comment 20 by pdr@chromium.org, Nov 24 2014

@tkent, @dsinclair, @jchaffraix, how do you all feel about the suggestion in #19?
Making our implementation match the other browsers makes sense to me. We should kick off an email to the spec list to see about getting the spec updated to match the implementations.

Comment 22 by tkent@chromium.org, Nov 25 2014

Definitely we should discuss this issue in a standard body.
I don't think we need to change Blink behavior immediately.  This behavior is behind the flag, and doesn't affect a lot of sites.


Comment 23 by pdr@chromium.org, Nov 25 2014

I apologize... I always have the experimental web platform features flag enabled and thought this was affecting all users. My comments above were dire sounding because I thought we were shipping this.

I agree with tkent, please bring this up in a standards body. No need to revert this patch.

Comment 25 by tkent@chromium.org, Aug 25 2015

Blocking: chromium:87553

Comment 26 by tkent@chromium.org, Aug 25 2015

Labels: Needs-Evangelism
Status: ExternalDependency
Labels: Hotlist-Recharge
This issue likely requires triage.  The current issue owner may be inactive (i.e. hasn't fixed an issue in the last 30 days or commented in this particular issue in the last 90 days).  Thanks for helping out!

-Anthony
Labels: Cr-Blink-Layout
Owner: e...@chromium.org
The site still shows up as broken for me. Looks like the spec discussion didn't arrive at any conclusion.

eae@ is this something layout team would be responsible for?

Comment 29 by e...@chromium.org, Oct 5 2015

Cc: tabatkins@chromium.org
Tab, any chance we could get this clarified form a spec perspective?

I just pinged the thread again.  I'm like 80% sure this page is just *coincidentally* using elements named <menu>/<menuitem>, and they *mean* them to be unknown elements.  The markup is just stupidly nonsensical otherwise.

If so, this is just an evangelism bug.

Comment 31 by e...@chromium.org, Oct 7 2015

Ah, that makes sense and matches my analysis of the page. Thanks Tab!

Comment 32 by zcorpan@gmail.com, Oct 7 2015

See https://github.com/whatwg/html/issues/234

There is a proposal to make this work, but it is a bit more complicated and prior research suggests that this is not a common problem, so hopefully evang will be enough. Please comment in the issue if you find more broken pages or evang fails etc.
Cc: ashej...@chromium.org pbomm...@chromium.org sanjoy....@samsung.com
 Issue 550519  has been merged into this issue.
Labels: -M-39 -Cr-Blink-Layout Cr-Blink-HTML-Parser
Labels: -Type-Bug-Regression -Hotlist-Recharge Type-Bug
Owner: ----
Project Member

Comment 36 by bugdroid1@chromium.org, Jan 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/960e27eea0a509153ee247d7edda7d1c4d49eb53

commit 960e27eea0a509153ee247d7edda7d1c4d49eb53
Author: tkent <tkent@chromium.org>
Date: Thu Jan 14 06:48:01 2016

Add counters for <menuitem>.

- UseCounter::MenuItemElement
  Counts menuitem element usage.
  if chrome://flags/#enable-experimental-web-platform-features is enabled,
  HTMLMenuItemElement is used.  Otherwise, HTMLUnknownElement is used.

- UseCounter::MenuItemCloseTag
  Counts </menuitem>.  <menuitem> is a void element according to the current
  specification.  This counts wrong usages like  crbug.com/412945 .

BUG= 412945 

Review URL: https://codereview.chromium.org/1587673005

Cr-Commit-Position: refs/heads/master@{#369354}

[modify] http://crrev.com/960e27eea0a509153ee247d7edda7d1c4d49eb53/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] http://crrev.com/960e27eea0a509153ee247d7edda7d1c4d49eb53/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp
[modify] http://crrev.com/960e27eea0a509153ee247d7edda7d1c4d49eb53/third_party/WebKit/Source/core/html/HTMLUnknownElement.cpp
[modify] http://crrev.com/960e27eea0a509153ee247d7edda7d1c4d49eb53/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp

Comment 37 by sim...@opera.com, Mar 30 2016

Spec change PR: https://github.com/whatwg/html/pull/907

html5lib-tests PR: https://github.com/html5lib/html5lib-tests/pull/72

These PRs are currently awaiting review. If someone here has knowledge and time to take a look, that would be much appreciated.

(Will import the tests to web-platform-tests when they land in html5lib-tests.)
Cc: majidvp@chromium.org
 Issue 601631  has been merged into this issue.
Cc: -eseidel@chromium.org -jchaffraix@chromium.org
Summary: Apple API reference page rendering is very broken with 'Experimental Web Platform Features' (was: HealthKit API rendering is very broken)
Labels: -Needs-Evangelism Hotlist-Interop
Owner: tkent@chromium.org
Status: Available (was: ExternalDependency)
The spec has been changed to mark menuitem as options-like instead of a void element: https://github.com/whatwg/html/pull/907

Can we please get chromium's implementation fixed so that the Apple docs aren't all messed up when experimental web platform features is enabled (or, worse case, move the <menuitem> support out of experimental)?
Components: -Blink>HTML
Owner: ----

Comment 43 by kochi@chromium.org, May 31 2016

BTW,  I bumped into apple site breakage today (see attachment),
visiting
https://developer.apple.com/search/?q=threading&type=Documentation
on Canary (53.0.2751.0 (Official Build) canary (64 bit)) with
"experimental-web-platform-features" on.

The search results are all blurry, though you can click on it or select the snippets.

Screenshot 2016-05-31 14.30.44.png
489 KB View Download

Comment 44 by phistuck@gmail.com, May 31 2016

#43 -
That is a totally different issue. It is related to position: sticky, or to backdrop-filter. Can you file a new issue?

Comment 45 by kochi@chromium.org, May 31 2016

Oh sure, filed  issue 615957 
Owner: rbyers@chromium.org
Status: Started (was: Available)
Given that (after this bug has been open for 2 years) we don't have a concrete plan for preventing this breakage, and chromium developers REALLY need to be able to use the apple developer website, I intend to downgrade the ContextMenu RuntimeEnabledFeature from status=experimental to status=test.

When either the Apple site is fixed, or the implementation is changed to not break, we can re-promote it to experimental.

Sorry instead of "chromium developers", what I really meant was ALL people that tend to run with --enable-experimental-web-platform-features (which is a much larger set).  We can't have any experimental feature cause long-lasting breakage on major sites.
Project Member

Comment 48 by bugdroid1@chromium.org, Jun 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/990b0dfe5c98990f3a27e44a2a5e512f5ed3140d

commit 990b0dfe5c98990f3a27e44a2a5e512f5ed3140d
Author: rbyers <rbyers@chromium.org>
Date: Thu Jun 09 03:31:03 2016

Remove HTML5 menu tag feature from experimental set

There are website compat issues with this feature on sites that use their
own <menuitem> tags (especially developer.apple.com).  Remove it from
the "experimental" set until there's a concrete plan in place to either
ship or resolve the issue to prevent ongoing issues for users running
with this flag enabled.

Note that you can always run chromium with
--enable-blink-features=ContextMenu in order to get this feature.

BUG= 412945 

Review-Url: https://codereview.chromium.org/2049093003
Cr-Commit-Position: refs/heads/master@{#398771}

[modify] https://crrev.com/990b0dfe5c98990f3a27e44a2a5e512f5ed3140d/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Status: Fixed (was: Started)
Cc: -le...@chromium.org
Status: Available (was: Fixed)
I'm moving this back to available, it is blocking the Menu item launch and, since the underlying issue isn't fixed, we should keep it open to make sure we don't loose track next time we try to enable Menu.
Has Apple said anything about fixing their site?

Comment 52 by phistuck@gmail.com, Jun 26 2016

Has anyone filed a bug report ("radar"?) with Apple (not WebKit, as it is unrelated)?

Comment 53 by sim...@opera.com, Jun 27 2016

Cc: ho...@apple.com
sanjoy.pal@ said in https://github.com/whatwg/html/pull/907#issuecomment-226106645

> I filed a bug in apple bug tracker but no action is taken.

I've asked hober (Apple) to follow up with the Apple docs people, he said he would do so last week.
Cc: rbyers@chromium.org
Owner: ----
Summary: Apple API reference page rendering is very broken with "ContextMenu" feature enabled (was: Apple API reference page rendering is very broken with 'Experimental Web Platform Features')

Comment 55 by zcorpan@gmail.com, Sep 16 2016

The affected URL no longer uses `menuitem`.
Given that the Apple API page no longer (mis)uses `menuitem`, can we re-add ContextMenu to the experimental feature set?
If there's someone interested in driving it to ship, then re-adding it is fine with me.  But if we don't have any concrete plans to ship it, then I don't think it's worthwhile to call it "experimental".  

Comment 58 by zcorpan@gmail.com, Nov 28 2016

FYI it appears Mozilla are implementing the parser spec changes now,
https://bugzilla.mozilla.org/show_bug.cgi?id=676236
Firefox is fine with:

    <menuitem id="cmd-copy" label="Copy">Copy</menuitem>

but it only uses the label attribute. It'd be nice to keep the node value for semi-polyfilling the element. Firefox is the only one that understands <menuitem> and it doesn't care about the node value. That's perfect, because Chrome doesn't understand <menuitem label>, but it does understand CSS.

I've been hoping for this feature for 5 years now =) So very very useful for mobile context menus.
Status: ExternalDependency (was: Available)
It seems Mozilla is asking to change the parsing rule.
https://github.com/whatwg/html/issues/2308

Comment 62 by tkent@chromium.org, Mar 23 2017

Status: Available (was: ExternalDependency)
Summary: <menuitem> parsing (was: Apple API reference page rendering is very broken with "ContextMenu" feature enabled)

Comment 63 by tkent@chromium.org, Mar 23 2017

Summary: <menuitem> parsing is no web-compatible (was: <menuitem> parsing)

Comment 64 by yuzus@chromium.org, Apr 13 2017

Owner: yuzus@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 65 by bugdroid1@chromium.org, Apr 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90d6ed690cab69c72e9a8994ed209f7b3432e44c

commit 90d6ed690cab69c72e9a8994ed209f7b3432e44c
Author: yuzus <yuzus@chromium.org>
Date: Mon Apr 17 08:49:57 2017

Change <menuitem> parsing rules to match spec

This CL changes <menuitem> parsing rules so that they match the current spec.
<menuitem> is no more a self closing tag.

The link below shows the latest change made to the spec.
https://github.com/whatwg/html/pull/2319

BUG= 412945 

Review-Url: https://codereview.chromium.org/2819773002
Cr-Commit-Position: refs/heads/master@{#464906}

[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/LayoutTests/external/wpt/html/syntax/serializing-html-fragments/serializing-expected.txt
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/LayoutTests/external/wpt/html/syntax/serializing-html-fragments/serializing.html
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/LayoutTests/fast/parser/parse-menuitem-expected.txt
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/LayoutTests/fast/parser/parse-menuitem.html
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-click.html
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/Source/core/html/HTMLElement.cpp
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/Source/core/html/parser/HTMLStackItem.h
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp
[modify] https://crrev.com/90d6ed690cab69c72e9a8994ed209f7b3432e44c/third_party/WebKit/Source/core/page/ContextMenuControllerTest.cpp

Comment 66 by yuzus@chromium.org, Apr 17 2017

Status: Fixed (was: Assigned)

Sign in to add a comment