Bug 103172 – implement the HTML <main> element

• Component: DOM
• Status: RESOLVED FIXED

[Steve Faulkner]

<main> should behave essentially the same as <div> for parsing etc. 

the changes will be the same as for the implementaion of the <nav> element
https://bugs.webkit.org/attachment.cgi?id=35098&action=prettypatch

implement the HTML <main> element by making the appropriate changes to:
WebCore/css/html.css
WebCore/editing/htmlediting.cpp
WebCore/html/HTMLElement.cpp
WebCore/html/HTMLParser.cpp
WebCore/html/HTMLTagNames.in

[Steve Faulkner]

once implemented also need to map <main> to main acc role

[Steve Faulkner]

Created attachment 175877 [details]
patch to add <main> 

changes made to add main support

mapped LandamrkMainRole to mainTag
[WebCore\accessibility\AccessibilityRenderObject.cpp]
Line 2485 : 	if (node && node->hasTagName(mainTag))
Line 2486 :     return LandmarkMainRole;

added mainTag as a blockTag
[WebCore\WebKit\editing\FormatBlockCommand.cpp]
Line 139 : 		blockTags.add(mainTag);
added mainTag as a stackItem
[WebCore\WebKit\html\parser\HTMLStackItem.h]
Line 171 : 			|| tagName == HTMLNames::mainTag

added mainTag in tree builder
[WebCore\WebKit\html\parser\HTMLTreeBuilder.cpp]
Line 714 : 		|| token->name() == mainTag
Line 1726 : 		|| token->name() == mainTag

added main to following style declaration:
[WebCore\css\html.css]
article, aside, footer, header, hgroup, main, nav, section {
    display: block
}

[Steve Faulkner]

(In reply to comment #0)
> <main> should behave essentially the same as <div> for parsing etc. 
> 
> the changes will be the same as for the implementaion of the <nav> element
> https://bugs.webkit.org/attachment.cgi?id=35098&action=prettypatch
> 
> implement the HTML <main> element by making the appropriate changes to:
> WebCore/css/html.css
> WebCore/editing/htmlediting.cpp
> WebCore/html/HTMLElement.cpp
> WebCore/html/HTMLParser.cpp
> WebCore/html/HTMLTagNames.in

seems that changes to some of above no longer elevant

[Steve Faulkner]

(In reply to comment #1)
> once implemented also need to map <main> to main acc role

added main mapping in patch

[Steve Faulkner]

Created attachment 175879 [details]
patch to add <main> v1

fixed formatting

[Steve Faulkner]

Comment on attachment 175879 [details]
patch to add <main> v1

fixed mimetype

[Adam Barth]

Comment on attachment 175879 [details]
patch to add <main> v1

This patch is missing a ChangeLog and tests.  Please see http://www.webkit.org/coding/contributing.html for instructions on how to contribute to WebKit.  Also, please don't set the review flag to +.  That means that a WebKit reviewer has approved your patch.  Instead, you should set the review flag to ?, which indicates that you would like a reviewer to look at your patch.

[Adam Barth]

My understanding is that this feature is somewhat controversial in the WHATWG.

[Steve Faulkner]

(In reply to comment #7)
> (From update of attachment 175879 [details])
> This patch is missing a ChangeLog and tests.  Please see http://www.webkit.org/coding/contributing.html for instructions on how to contribute to WebKit.  Also, please don't set the review flag to +.  That means that a WebKit reviewer has approved your patch.  Instead, you should set the review flag to ?, which indicates that you would like a reviewer to look at your patch.

ok thanks for the info, first time so unsure of what to do

[Steve Faulkner]

(In reply to comment #8)
> My understanding is that this feature is somewhat controversial in the WHATWG.

It is only controversial in as much as Hixie does not agree with it. Otherwise it has no major objections and support form a a range of participants. I have provided a patch on the basis of Maciej's email: http://lists.w3.org/Archives/Public/public-whatwg-archive/2012Nov/0104.html

[Adam Barth]

> ok thanks for the info, first time so unsure of what to do

We've all contributed for the first time, so we understand.  :)

[Ian 'Hixie' Hickson]

Personally I don't care one way or the other; the reason it's not in the WHATWG spec isn't about my opinion, it's about the lack of compelling use cases. Adding a feature to the platform is expensive. Adding a feature which is likely to be misused (as determined by looking at how people use class=main now, compared to the proposed definition of the element) just pollutes the Web for no good reason.

In the case of this patch the danger is even worse; it is likely to actually make accessibility worse. The patch makes <main> act like role=main, but this will cause the many pages that will misuse it to report less useful results to AT users than if the element wasn't there. There are other mechanisms that have been suggested for similar results, which are less likely to be misused.

[Steve Faulkner]

(In reply to comment #12)
> Personally I don't care one way or the other; the reason it's not in the WHATWG spec isn't about my opinion, it's about the lack of compelling use cases. Adding a feature to the platform is expensive. Adding a feature which is likely to be misused (as determined by looking at how people use class=main now, compared to the proposed definition of the element) just pollutes the Web for no good reason.
> 
> In the case of this patch the danger is even worse; it is likely to actually make accessibility worse. The patch makes <main> act like role=main, but this will cause the many pages that will misuse it to report less useful results to AT users than if the element wasn't there. There are other mechanisms that have been suggested for similar results, which are less likely to be misused.

Making unsubstantiated  statements  such as much of the above does little to promote your view.

What you say are your opinions and not facts, and have not been accepted by others, who have reviewed and commented on the the data, the rationale, uses cases and the spec I wrote and have found them to have merit.

[Ian 'Hixie' Hickson]

Nobody said your data and so forth didn't have merit, just that on the balance you reached the wrong conclusion.

(Incidentally, since people on the WHATWG list are encouraged to not post comments along the line of "+1" and since people have seen that I take a relatively reasonable view, people will often just assume that I'm going to reject bad suggestions and not comment. So saying that I'm the only one who has said anything negative on the thread isn't evidence that I'm the only person who reached that conclusion — it's my job to be the one who's negative. You can draw more conclusions from the fact that nobody else is implementing this feature, IMHO. It's not like people don't implement stuff I don't spec — it happens all the time, even without a spec at all. This feature has a spec, even, and still doesn't have implementors. It's not generally a sign of support that the same person who's writing the spec also has to write the code to put it in browsers. Don't confuse indifference for support. But none of this is relevant — what matters isn't how much support something has, it's how much justification it has.)

[Steve Faulkner]

(In reply to comment #14)
> Nobody said your data and so forth didn't have merit, just that on the balance you reached the wrong conclusion.
> 
> (Incidentally, since people on the WHATWG list are encouraged to not post comments along the line of "+1" and since people have seen that I take a relatively reasonable view, people will often just assume that I'm going to reject bad suggestions and not comment. So saying that I'm the only one who has said anything negative on the thread isn't evidence that I'm the only person who reached that conclusion — it's my job to be the one who's negative. You can draw more conclusions from the fact that nobody else is implementing this feature, IMHO. It's not like people don't implement stuff I don't spec — it happens all the time, even without a spec at all. This feature has a spec, even, and still doesn't have implementors. It's not generally a sign of support that the same person who's writing the spec also has to write the code to put it in browsers. Don't confuse indifference for support. But none of this is relevant — what matters isn't how much support something has, it's how much justification it has.)


The spec has been around a short time, a few months at most, so now you have moved from being in disagreement to being disingenuous , which is unfortunate.

The thread started by Simon Pieters on whatwg [1] 

started with an observation:

"My impression from TPAC is that implementors are on board with the idea of  
adding <main> to HTML, and we're left with Hixie objecting to it."

and ended with a specific questions:

"If there is anyone besides from Hixie who objects to adding <main>, it  
would be useful to hear it."

While there were many responses there were no other objections.


When I tweeted today that I had submitted a patch, the response from Marco Zehe one of the mozilla accessibility engineers: 


"Want to create a Firefox patch, too?" [2]


[1] http://lists.w3.org/Archives/Public/public-whatwg-archive/2012Nov/0055.html
[2] https://twitter.com/MarcoInEnglish/status/272648449483218944

[Maciej Stachowiak]

In addition to what Adam said about tests and a ChangeLog entry, this is a new feature, so it should be discussed on webkit-dev to determine if the WebKit community agrees with adding it. The details for that are described here: <http://www.webkit.org/coding/adding-features.html>.

If there's a need for discussion about whether the <main> feature is a good idea, then please let's have it on the webkit-dev thread, not here in the bug.

[Maciej Stachowiak]

(In reply to comment #15)
> 
> The spec has been around a short time, a few months at most, so now you have moved from being in disagreement to being disingenuous , which is unfortunate.
> 

The WebKit community has high standards for courteous communication, whether in email or in the bug tracker. Please let's avoid attacks on others' integrity and indeed personal attacks of any kind.

[Steve Faulkner]

(In reply to comment #17)
> (In reply to comment #15)
> > 
> > The spec has been around a short time, a few months at most, so now you have moved from being in disagreement to being disingenuous , which is unfortunate.
> > 
> 
> The WebKit community has high standards for courteous communication, whether in email or in the bug tracker. Please let's avoid attacks on others' integrity and indeed personal attacks of any kind.

My apologies for using the word disingenuous in the bug report

[Steve Faulkner]

(In reply to comment #16)
> In addition to what Adam said about tests and a ChangeLog entry, this is a new feature, so it should be discussed on webkit-dev to determine if the WebKit community agrees with adding it. The details for that are described here: <http://www.webkit.org/coding/adding-features.html>.
> 
> If there's a need for discussion about whether the <main> feature is a good idea, then please let's have it on the webkit-dev thread, not here in the bug.

I have signed up for the mailing list once. I have access will send the appropriate email.

[Steve Faulkner]

Created attachment 175960 [details]
v2 of patch for main - no tests yet

double checked previous changes and new addition to HTMLTagNames.in

[Steve Faulkner]

With help, got a chromium/Mac build with patch running. Adhoc testing of layout tests https://dl.dropbox.com/u/377471/main2.html all pass and <main> is exposed as
AXRole:"AXGroup"
AXSubRole:"AXLandmarkMain"
AXRoleDescription:"Group"

note: role description should be "Main", but Chromium does not appear to expose specific AXRoleDescription for majority of new HTML5 structural elements: <nav> <header> etc, although the AXSubRole values are correct.

[chris fleizach]

Comment on attachment 175960 [details]
v2 of patch for main - no tests yet

looks ok.
are you having any trouble making a layout test for accessibility to test this?

[Eric Seidel (no email)]

The consensus from the webkit-dev discussion on this topic was that this needs further spec-work from the whatwg.

Please feel free to file a new bug if this is later if needed.

Thanks!

http://lists.webkit.org/pipermail/webkit-dev/2012-November/022991.html

[sideshowbarker]

Created attachment 176418 [details]
complete patch with tests and ChangeLog files

[Maciej Stachowiak]

(In reply to comment #23)
> The consensus from the webkit-dev discussion on this topic was that this needs further spec-work from the whatwg.
> 
> Please feel free to file a new bug if this is later if needed.
> 
> Thanks!
> 
> http://lists.webkit.org/pipermail/webkit-dev/2012-November/022991.html

Seems not unlikely that will be the outcome, but maybe we should give it a few more days, since folks have spoken up on both sides of the issue?

[Steve Faulkner]

(In reply to comment #25)
> (In reply to comment #23)
> > The consensus from the webkit-dev discussion on this topic was that this needs further spec-work from the whatwg.
> > 
> > Please feel free to file a new bug if this is later if needed.
> > 
> > Thanks!
> > 
> > http://lists.webkit.org/pipermail/webkit-dev/2012-November/022991.html
> 
> Seems not unlikely that will be the outcome, but maybe we should give it a >few more days, since folks have spoken up on both sides of the issue?


>The consensus from the webkit-dev discussion on this topic was that this >needs further spec-work from the whatwg.

Hi Maciej, some advice here would be useful:

Besides the fact that the spec work is not being carried out at the WHATWG.
What further spec work needs to go on a the WHATWG?
The discussion has been effectively shut down at the WHATWG by hixie i.e. unless new data is provided that convinces him to change his mind. 

There has been no new data offered or technical argument on the spec on the webkit-dev, only hixies re-assertions that were not backed up with anything new by him and were not convincing when discussed on the WHATWG list. Have I have missed any substantive feedback posted to any lists? 

So what would you suggest are the next steps?

[James Craig]

This extension specification was approved "with no objections and ample support" by the main HTML Working Group. http://lists.w3.org/Archives/Public/public-html/2012Nov/0232.html

Which means inclusion of the patch should be accepted once it gets reviewer approval. 

As the standards discussion continues within W3C and WHATWG, additional bugs and patches may need to be added to support future changes to the specification.

[chris fleizach]

Comment on attachment 176418 [details]
complete patch with tests and ChangeLog files

we'll still want an accessibility test as well

[sideshowbarker]

(In reply to comment #28)
> (From update of attachment 176418 [details])
> we'll still want an accessibility test as well

I'll contribute one to the patch tomorrow.

[sideshowbarker]

Reopening to attach new patch.

[sideshowbarker]

Created attachment 178901 [details]
Patch

[sideshowbarker]

Updated patch adds an accessibility layout test.

[Eric Seidel (no email)]

Comment on attachment 178901 [details]
Patch

Cleared review? from attachment 178901 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).

[sideshowbarker]

Reopening to attach new patch.

[sideshowbarker]

Created attachment 182645 [details]
Patch

[sideshowbarker]

Reset review? flag per discussion with Chris Fleizach; please hold off on clearing it until Chris has had the opportunity to follow up.

[WebKit Review Bot]

Comment on attachment 182645 [details]
Patch

Attachment 182645 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15870737

New failing tests:
editing/execCommand/query-format-block.html
fast/dom/wrapper-classes.html

[sideshowbarker]

(In reply to comment #37)
> (From update of attachment 182645 [details])
> Attachment 182645 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15870737
> 
> New failing tests:
> editing/execCommand/query-format-block.html
> fast/dom/wrapper-classes.html

I'm fixing these now and when I have them passing will upload an updated patch.

[Build Bot]

Comment on attachment 182645 [details]
Patch

Attachment 182645 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15860756

New failing tests:
editing/execCommand/query-format-block.html
accessibility/main-element.html

[sideshowbarker]

Created attachment 182695 [details]
Patch

Fixed failing ests: fixed fast/dom/wrapper-classes.html test for chromium; fixed editing/execCommand/query-format-block.html test for chromium and mac; did not yet fix accessibility/main-element.html test failure on mac -- will follow up with Chris Fleizach to figure out why it's failing.

[James Craig]

It looks like your expectation is that the AXRole will be AXLandmarkMain, which will actually be the AXSubrole. You should still get a generic AXGroup for the role.

[sideshowbarker]

(In reply to comment #41)
> It looks like your expectation is that the AXRole will be AXLandmarkMain, which will actually be the AXSubrole. You should still get a generic AXGroup for the role.

Thanks James. So I'm wondering if that failure is due to differences in what's exposed by the chromium port vs the mac port. In my chromium build environment, that test passes with the expected result of AXLandmarkMain for AXRole.

[James Craig]

That's interesting. I'm not sure about the answer then.

Check out the rest of that IDL interface in ./Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl

If you have a Mac, you can also verify the AX API using the Accessibility Inspector mentioned here:
http://developer.apple.com/library/mac/#documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXTesting/OSXAXTestingApps.html

[chris fleizach]

(In reply to comment #42)
> (In reply to comment #41)
> > It looks like your expectation is that the AXRole will be AXLandmarkMain, which will actually be the AXSubrole. You should still get a generic AXGroup for the role.
> 
> Thanks James. So I'm wondering if that failure is due to differences in what's exposed by the chromium port vs the mac port. In my chromium build environment, that test passes with the expected result of AXLandmarkMain for AXRole.

Chromium and Mac can have different roles like this. Your test could check the platform name property. You can also look at some of the other landmark tests and see if they do something else

[Build Bot]

Comment on attachment 182695 [details]
Patch

Attachment 182695 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15884533

New failing tests:
accessibility/main-element.html

[Build Bot]

Comment on attachment 182695 [details]
Patch

Attachment 182695 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15871871

New failing tests:
accessibility/main-element.html

[Build Bot]

Comment on attachment 182695 [details]
Patch

Attachment 182695 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15903335

New failing tests:
accessibility/main-element.html

[Ryosuke Niwa]

Comment on attachment 182695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182695&action=review

> Source/WebCore/ChangeLog:1
> [phone]  Steve Faulkner  <[e-mail]>

Please change the author to yourself and just mention that it's co-authored by Steve Faulkner in the change log below.

[chris fleizach]

Comment on attachment 182695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182695&action=review

> LayoutTests/accessibility/main-element.html:19
> +          shouldBe("main.role", "'AXRole: AXLandmarkMain'");

what you want here for the mac platform is

shouldBe("main.role", "'AXRole: AXGroup'");
shouldBe("main.subrole", "'AXSubrole: AXLandmarkMain'");

[sideshowbarker]

Created attachment 183453 [details]
Patch

Removed failing platform-unaware ax test and replaced it with separate platform-specific ax tests (for chromium and mac).

[Dominic Mazzoni]

Comment on attachment 183453 [details]
Patch

Unofficial review, accessibility looks okay with a couple of minor nits. Tests pass so this should be pretty close.

View in context: https://bugs.webkit.org/attachment.cgi?id=183453&action=review
> LayoutTests/platform/chromium/accessibility/main-element.html:21
> +          shouldBe("body.childAtIndex(0).role", "'AXRole: AXLandmarkMain'");

This is okay, but check out accessibilityController.accessibleElementById for a way to do this without needing to assume that the element you want is the first child of the body. (Most of the more recent tests use this pattern instead, it's more robust.)

> LayoutTests/platform/mac/accessibility/main-element.html:21
> +          shouldBe("body.childAtIndex(0).subrole", "'AXSubrole: AXLandmarkMain'");

This is good, but I would test the role too.

[chris fleizach]

Comment on attachment 183453 [details]
Patch

please try to address Dominic's comments. otherwise it's looking good. thanks

[sideshowbarker]

Created attachment 183608 [details]
Patch

Address Dominic's review feedback in comment #51 (use accessibilityController.accessibleElementById and test for both the role and subrole properties).

[chris fleizach]

Comment on attachment 183608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183608&action=review

> LayoutTests/platform/mac/accessibility/main-element-expected.txt:8
> +PASS main.subrole is 'AXRole: AXLandmarkMain'

this line should be 'AXSubrole: AXLandmarkMain'

that's prob why its failing

[chris fleizach]

Comment on attachment 183608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183608&action=review

> LayoutTests/platform/chromium/accessibility/main-element.html:23
> +          shouldBe("main.subrole", "undefined");

probably no need to include subrole on chromium since its undefined

[sideshowbarker]

(In reply to comment #54)
> > LayoutTests/platform/mac/accessibility/main-element-expected.txt:8
> > +PASS main.subrole is 'AXRole: AXLandmarkMain'
> 
> this line should be 'AXSubrole: AXLandmarkMain'

d'oh

> that's prob why its failing

yeah will fix it now

(In reply to comment #55)
> > LayoutTests/platform/chromium/accessibility/main-element.html:23
> > +          shouldBe("main.subrole", "undefined");
> 
> probably no need to include subrole on chromium since its undefined

OK, will drop it

[sideshowbarker]

Created attachment 183611 [details]
Patch

Fixed test flub and dropped check for ax subrole on chromium per comment #55 from Chris.

[WebKit Review Bot]

Comment on attachment 183611 [details]
Patch

Clearing flags on attachment: 183611

Committed r140341: <http://trac.webkit.org/changeset/140341>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 183611 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183611&action=review

> LayoutTests/fast/dom/wrapper-classes-expected.txt:346
> +PASS tagJSWrapperClass('main') is 'HTMLElement'
> +PASS tagJSWrapperPrototypeClass('main') is 'HTMLElementPrototype'
> +PASS tagJSWrapperConstructorClass('main') is 'HTMLElementConstructor'

Is this correct? Is this specified? Does this match the behavior in the other browsers?

[Maciej Stachowiak]

(In reply to comment #60)
> (From update of attachment 183611 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=183611&action=review
> 
> > LayoutTests/fast/dom/wrapper-classes-expected.txt:346
> > +PASS tagJSWrapperClass('main') is 'HTMLElement'
> > +PASS tagJSWrapperPrototypeClass('main') is 'HTMLElementPrototype'
> > +PASS tagJSWrapperConstructorClass('main') is 'HTMLElementConstructor'
> 
> Is this correct? Is this specified? Does this match the behavior in the other browsers?

The quoted results are correct per the HTML 5.1 editor's draft:
http://www.w3.org/html/wg/drafts/html/master/single-page.html#the-main-element

It doesn't match other browsers because none have implemented this new feature yet, though Gecko engineers plan to implement it, so it will likely soon be in Firefox.