591737 - Support for HTML5's <details> and <summary>

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Jean-Yves Perrier [:teoli]]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre Minefield/4.0b5pre
Build Identifier: 

A lot of sites needs to hide part of the web page and make them hide/unhide when we click on a button or a link.

HTML5 propose two new elements to allows this behaviour: <details> and <summary>.

These two elements not only conveys semantic information about the relation between a detail and its summary, but allows to show/hide the details without the need of (trivial) javascript.

For instance, things like the following could be replaced to drop JS:
<a href='#' onclick='toggleVisibility(0); return false' title='See détails (need JavaScript)'>
(This example line is used on wikipedia).


Reproducible: Always




Coupled with transition, this may also remove the need for some use of Flash (photo galleries?)

Comment 2

[Will Pittenger]

In my tests using the "Try it yourself" link at http://www.w3schools.com/html5/tag_details.asp, the <details> tag contents remain visible by default using Beta 8.  According to both that page and http://www.w3.org/TR/html5/interactive-elements.html#the-details-element the contents of <details> are supposed to be invisible unless the "open" attribute is specified.  This clearly isn't how Beta 8 is behaving.  Is this tag implemented yet?  Personally, the tag seems redundant, but I don't do the specs.

Comment 3

[Josh Tumath]

(In reply to comment #2)
No, this is not supported yet. :) If it were supported, this bug would be fixed.

At this moment in time, no layout engine is going to support the details element, because it is still an unstable part of the spec. It could change at any time. It could even be completely removed from the spec soon, because there is an issue as to whether or not the element is actually needed.

Comment 4

[funcod2]

@josh not anymore.

The problem has been settled you can start implementing it.

Comment 5

[Patrick Dark]

@Comment 4: Could you please cite a WHATWG/W3C discussion/resolution, a browser implementation, or other info supporting that claim? While I think that these elements should be implemented, I haven't seen anything to indicate that the status of these elements has changed.

Comment 6

[funcod2]

the issue:
http://www.w3.org/html/wg/tracker/issues/93
a bug:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=8379 (which confirms the decision)
the decision:
http://lists.w3.org/Archives/Public/public-html/2010Jun/att-0659/issue-93-decision.html

Comment 7

[funcod2]

The only question I have concerning the implementation is the scrollintoview expected/default behaviour.
The rest is really straightforward.

Comment 8

[aja+bugzilla]

FWIW, Google Chrome 12.0.706.0 canary build now has a details/summary disclosure widget.
Not sure how spec-compliant it is at this point, though.
E.g., summary is block instead of inline, no 40px margin-start.

Comment 9

[Jean-Yves Perrier [:teoli]]

Opera has started a thread on the WhatWG mailing list about the styling of <details>. The discussion may be useful: http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2011-April/031132.html

Comment 10

[funcod2]

Someone could add [parity-webkit] on the whiteboard?
https://bugs.webkit.org/show_bug.cgi?id=50309

Comment 11

[funcod2]

sorry wrong one
https://bugs.webkit.org/show_bug.cgi?id=51071

Comment 12

[Paul Rouget [:paul]]

Is this hard to fix? If not, could it be a [good first bug]?

Comment 13

[Mounir Lamouri (:mounir)]

(In reply to Paul Rouget [:paul] from comment #12)
> Is this hard to fix?

This is not trivial.

> If not, could it be a [good first bug]?

FWIW, [good first bugs] have been deprecated by mentored bugs.

Comment 14

[Rod Smith]

<details><summary> in Firefox/SeaMonkey/KompoZer would be very useful for me -- re-formatting large web-based manuals (e.g. Linux manuals) as active outlines, like those in the sub-directories of  http://www.rodericksmith.plus.com/outlines , (until now, they used JavaScript).  

I find them so much easier to browse for information than PDF, word, man pages or standard long web pages.

I'll be adding a limb of the directory tree with those outlines in pure HTML5 quite soon -- the pure HTML5 versions work with Google Chrome and with standard Android tablet browsers.  

With <details><summary>  creating the outlines will be easier too (once I update my outline reformatting tools).

Comment 15

[Cameron McCormack (:heycam)]

I've begun looking at this.  A few questions:

1. Do we want the disclosure widget to look appropriate for different platforms, or
   should we just draw a triangle on all platforms?  And if so, should we draw it
   with the colour of the <summary> element, as WebKit does?

2. Should just the disclosure widget be clickable, or the whole <summary> element's
   contents?

3. In WebKit, a big focus ring is displayed around the <summary>, and the element is
   focusable.  Should we be doing the same?  I don't really like the look of the
   big focus ring, but I think it's useful if you can tab to the element by default.

Comment 16

[Erik Rose [:erik][:erikrose]]

I lean strongly toward a platform-appropriate look for the widget, because (and I'd love to see metrics on this) people switch apps more often than they switch platforms. It's true of me, anyway.

Comment 17

[Rod Smith]

Disclosure widget: 
------------------

For outlining use, I'd prefer the option to provide one's own open/closed folder image, because that relates to older windows users' expectations of file browsers.

Additional capabilities  -- suggestions for further comment
-----------------------

onclick, onhover
----------------

Those options would reduce the JavaScript demands on more complex implementations (presenting partial details on hover, or monitoring what the end-user viewed and for how long)

Summary below details
---------------------

In making outlines, the caption of a figure or illustration (that you'd want to click for the full representation) should usually appear below the 'details' of the full figure. That's currently not possible in the Google implementation (and not mentioned in the HTML5 specification); irrespective of the placement of code in the details section, it appears below the summary display.

Alternate 'alt='
---------------

This would be useful e.g. for showing thumbprints that are replaced by full-sized images on clicking.


Now if we had <details><summary> implemented, think how much neater this comment would look!

Progress with implementation
----------------------------

I'm pleased to see some-one's assigned to the task now.  I (and possibly others of the commentators) would be willing beta-testers, and would appreciate news of progress as it happens!

Comment 18

[steve faulkner]

The HTML to Platform Accessibility APIs Implementation Guide provides recommendations on keyboard support and accessibility API mappings for the details/summary:

http://dvcs.w3.org/hg/html-api-map/raw-file/tip/Overview.html#examples-sum 
http://dvcs.w3.org/hg/html-api-map/raw-file/tip/Overview.html#summary-details
http://dvcs.w3.org/hg/html-api-map/raw-file/tip/Overview.html#el-130
http://dvcs.w3.org/hg/html-api-map/raw-file/tip/Overview.html#el-35

Comment 19

[Cameron McCormack (:heycam)]

(In reply to Rod Smith from comment #17)
> Disclosure widget: 
> ------------------
> 
> For outlining use, I'd prefer the option to provide one's own open/closed
> folder image, because that relates to older windows users' expectations of
> file browsers.

This sounds like a nice idea, but would need some spec work done.  There are many different ways that the disclosure triangle bit of the element is exposed to CSS.

> Additional capabilities  -- suggestions for further comment
> -----------------------
> 
> onclick, onhover
> ----------------
> 
> Those options would reduce the JavaScript demands on more complex
> implementations (presenting partial details on hover, or monitoring what the
> end-user viewed and for how long)

click, mouseover, etc. are are dispatched to all elements.  Or do you mean specifically to the disclosure triangle, and being able to detect that separately from the rest of the element?

> Summary below details
> ---------------------
> 
> In making outlines, the caption of a figure or illustration (that you'd want
> to click for the full representation) should usually appear below the
> 'details' of the full figure. That's currently not possible in the Google
> implementation (and not mentioned in the HTML5 specification); irrespective
> of the placement of code in the details section, it appears below the
> summary display.

Maybe; this wouldn't be hard to implement.  It could make interaction with the element difficult, though, as the <summary> bit and its disclosure triangle would move up and down in the page when you open or closer it.

> Alternate 'alt='
> ---------------
> 
> This would be useful e.g. for showing thumbprints that are replaced by
> full-sized images on clicking.

I don't really understand this one.

> Progress with implementation
> ----------------------------
> 
> I'm pleased to see some-one's assigned to the task now.  I (and possibly
> others of the commentators) would be willing beta-testers, and would
> appreciate news of progress as it happens!

I'll post try builds once I have something functional.

Comment 20

[Cameron McCormack (:heycam)]

Thanks for the links, Steve.(In reply to steve faulkner from comment #18)
> The HTML to Platform Accessibility APIs Implementation Guide provides
> recommendations on keyboard support and accessibility API mappings for the
> details/summary:

Thanks for those links.

Comment 21

[Cameron McCormack (:heycam)]

Attachment: WIP (v0.1)

This pretty much works.  Outstanding issues:
  * the accessibility mapping hasn't been done yet
  * a plain triangle is used as the disclosure widget on all platforms
  * clicking the triangle or <summary> contents can result in text selection,
    which looks a bit odd
  * the triangle is implemented with UA style rules on the <summary> making it
    display:list-item and with new -moz-disclosure-{open,closed} values for
    list-style-type, which is probably not the right solution
  * I haven't implemented anything in nsCSSFrameConstructor::CreateContinuingFrame
    for <details>, so it probably doesn't handle :first-letter properly

Comment 22

[Cameron McCormack (:heycam)]

Here are some try builds with that patch, if anyone is interested:

http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-6aea335df9b2/try-linux/firefox-20.0a1.en-US.linux-i686.tar.bz2
http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-6aea335df9b2/try-macosx64/firefox-20.0a1.en-US.mac.dmg
http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-6aea335df9b2/try-win32/firefox-20.0a1.en-US.win32.zip

Comment 23

[Cameron McCormack (:heycam)]

(In reply to Cameron McCormack (:heycam) from comment #21)
> This pretty much works.  Outstanding issues:

Oh, and no tests written yet. :)

Comment 24

[Michael[tm] Smith [:sideshowbarker]]

There seems to be a bug in the handling of nested <details>. Try this document:

https://gist.github.com/4316774

Click the triangle for the top-level details elements, then click on the triangle for the first nested details element.

What happens in my environment at least is, that details element you clicked on will then unexpectedly disappear (instead of expanding as expected to show its summary content).

Then click on the next two nested details and they'll disappear as well.

But if you click on the top-level triangle again, to unexpand it, and then click again to re-expand it, they'll all be back.

Also some nits: The disclosure triangle seems too big, and I wonder if it should be gray instead of black. At least the convention for disclosure triangles I've seen across most UIs is to make the gray.

Comment 25

[steve faulkner]

There is an issue with webkit implementation related to validation of required fields in details element https://twitter.com/getsetbro/status/280348413827104768

there is an example: http://jsfiddle.net/getsetbro/wtK4s/

Comment 26

[Cameron McCormack (:heycam)]

Attachment: WIP (v0.2)

(In reply to Michael[tm] Smith from comment #24)
> There seems to be a bug in the handling of nested <details>. Try this
> document:

Thanks, that should be fixed now.

> Also some nits: The disclosure triangle seems too big, and I wonder if it
> should be gray instead of black. At least the convention for disclosure
> triangles I've seen across most UIs is to make the gray.

I've made it a bit smaller (I was miscalculating the size before).  If we do platform specific rendering, then I'll make it grey (on Mac at least) then.


New builds:

http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-c071c7b6a382/try-linux/firefox-20.0a1.en-US.linux-i686.tar.bz2
http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-c071c7b6a382/try-macosx64/firefox-20.0a1.en-US.mac.dmg
http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-c071c7b6a382/try-win32/firefox-20.0a1.en-US.win32.zip

Comment 27

[Cameron McCormack (:heycam)]

(In reply to steve faulkner from comment #25)
> There is an issue with webkit implementation related to validation of
> required fields in details element
> https://twitter.com/getsetbro/status/280348413827104768
> 
> there is an example: http://jsfiddle.net/getsetbro/wtK4s/

Whether the <details> should automatically open to show the required field is an interesting question.  With my patch, you still get the "You need to fill out this field" popup, but it isn't positioned correctly -- it's placed outside the whole browser window.

You've got the same issue with any display:none form field that requires validation, by the way: http://jsfiddle.net/VsdHf/  At least with <details>, it seems reasonable to open them up to show the field that is failing validation.  For arbitrarily display:none styled form elements, we can't just show them.

Comment 28

[Michael[tm] Smith [:sideshowbarker]]

(In reply to Cameron McCormack (:heycam) from comment #26)
> (In reply to Michael[tm] Smith from comment #24)
> > There seems to be a bug in the handling of nested <details>. Try this
> > document:
> 
> Thanks, that should be fixed now.

Thanks, yeah, working as expected now.

> > Also some nits: The disclosure triangle seems too big, and I wonder if it
> > should be gray instead of black. At least the convention for disclosure
> > triangles I've seen across most UIs is to make the gray.
> 
> I've made it a bit smaller (I was miscalculating the size before). 

Yeah, looks better now.

> If we do
> platform specific rendering, then I'll make it grey (on Mac at least) then.

OK

One other thing: Mathias Byens has a details+summary demo page at http://mathiasbynens.be/demo/html5-details-jquery It implements a fallback mechanism for browsers that don't have native support rendering those elements yet. It correctly detects that the Gecko build I'm using does have native support, and works as expected -- except that it doesn't display the disclosure triangles. But if I disable his CSS stylesheet, the triangles are displayed as expected. It could be that there's something WebKit-specific in his CSS (I haven't checked yet but I will), but if not that, maybe something that your implementation doesn't expect and doesn't handle right. I guess it's more likely that his CSS is borked, since he couldn't test it with Gecko back when he created that page.

Comment 29

[Michael[tm] Smith [:sideshowbarker]]

(In reply to Michael[tm] Smith from comment #28)
> One other thing: Mathias Byens has a details+summary demo page at
> http://mathiasbynens.be/demo/html5-details-jquery ...
> ... it doesn't display the
> disclosure triangles.

The cause seems to be just that he has an explicit "details, summary { display: block }" in his stylesheet. Clearly it's not needed for browsers that have native support, but it seems like it at least should not prevent the triangle from being displayed in the case where some document does specify it for whatever reason.

Comment 30

[Cameron McCormack (:heycam)]

Yeah, that's because I have

  details > summary:first-of-type { display: list-item }

in the UA style sheet, and using the list bullet to render the disclosure triangle.  I'll resurrect the thread mentioned in comment 9 to discuss how exactly it should be exposed to CSS.

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Cameron McCormack (:heycam) from comment #15)
> 1. Do we want the disclosure widget to look appropriate for different
> platforms, or
>    should we just draw a triangle on all platforms?  And if so, should we
> draw it
>    with the colour of the <summary> element, as WebKit does?

We probably should draw the platform disclosure triangle. That would mean we'd want a pseudo-element where we can set -moz-appearance:XYZ.

We can draw animated themed widgets, so we might want to consider animating this too, although probably not in the first patch.

> 2. Should just the disclosure widget be clickable, or the whole <summary>
> element's
>    contents?

All the contents, although preventDefault() on the mouse event should stop that.

> 3. In WebKit, a big focus ring is displayed around the <summary>, and the
> element is
>    focusable.  Should we be doing the same?  I don't really like the look of
> the
>    big focus ring, but I think it's useful if you can tab to the element by
> default.

For buttons, we draw the focus ring if you tabbed into it with the keyboard, but not if you click with the mouse. You probably want to do the same thing here.

Comment 32

[Cameron McCormack (:heycam)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #31)
> We probably should draw the platform disclosure triangle. That would mean
> we'd want a pseudo-element where we can set -moz-appearance:XYZ.

What should we use for this frame?  Currently it's an nsBulletFrame that nsBlockFrame creates just due to the normal display:list-item behaviour.  I'd like to re-use the layout that gets done for nsBulletFrames -- is it OK to have it be an actual nsBulletFrame, if I can get that to paint properly when it has a -moz-appearance applied to it?

And should we use display:list-item as the actual way for the disclosure widget to be created?  It's really the only way (through CSS) to opt in to a marker frame being created.

If this is OK, then we could have this in the UA style sheet:

  details > summary:first-of-type {
    display: list-item;
  }

  details > summary:first-of-type::-moz-list-bullet {
    -moz-appearance: treetwisty; /* or something new if that's inappropriate */
    list-style-type: -moz-disclosure-closed;
  }

  details[open] > summary:first-of-type::-moz-list-bullet {
    list-style-type: -moz-disclosure-open;
  }

and then if the authors uses -moz-appearance:none they'll still get a plain looking triangle that they could then override.  They would need to know that it's a list-style-type for overriding, though.

(Eventually this should be ::marker rather than ::-moz-list-bullet I guess.)

> We can draw animated themed widgets, so we might want to consider animating
> this too, although probably not in the first patch.
> 
> > 2. Should just the disclosure widget be clickable, or the whole <summary>
> > element's
> >    contents?
> 
> All the contents, although preventDefault() on the mouse event should stop
> that.

I've done this, so it should be working.

Should we be someone stopping text selection from these mouse events too?  Double clicking on a <summary> results in its text being selected in addition to the details opening and then closing.  Clicking the disclosure widget also selects the contents of the <summary>, just like when you click a regular bullet.  What would be the proper way to inhibit this?


> > 3. In WebKit, a big focus ring is displayed around the <summary>, and the
> > element is
> >    focusable.  Should we be doing the same?  I don't really like the look of
> > the
> >    big focus ring, but I think it's useful if you can tab to the element by
> > default.
> 
> For buttons, we draw the focus ring if you tabbed into it with the keyboard,
> but not if you click with the mouse. You probably want to do the same thing
> here.

OK, that should be working too.  We get a dotted outline focus ring when focused rather than a big honking blue ring, which is good.

Comment 33

[brunoais]

(In reply to Cameron McCormack (:heycam) from comment #32)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #31)
> > We probably should draw the platform disclosure triangle. That would mean
> > we'd want a pseudo-element where we can set -moz-appearance:XYZ.
> 
> What should we use for this frame?  Currently it's an nsBulletFrame that
> nsBlockFrame creates just due to the normal display:list-item behaviour. 
> I'd like to re-use the layout that gets done for nsBulletFrames -- is it OK
> to have it be an actual nsBulletFrame, if I can get that to paint properly
> when it has a -moz-appearance applied to it?
> 
> And should we use display:list-item as the actual way for the disclosure
> widget to be created?  It's really the only way (through CSS) to opt in to a
> marker frame being created.
> 
> If this is OK, then we could have this in the UA style sheet:
> 
>   details > summary:first-of-type {
>     display: list-item;
>   }
> 
>   details > summary:first-of-type::-moz-list-bullet {
>     -moz-appearance: treetwisty; /* or something new if that's inappropriate
> */
>     list-style-type: -moz-disclosure-closed;
>   }
> 
>   details[open] > summary:first-of-type::-moz-list-bullet {
>     list-style-type: -moz-disclosure-open;
>   }

Please don't. Make it a block and not a list item. It also does not seem to be a list item. I don't know the limitations the native ff js & CSS has but I think there should be a way of doing it without using the display as list-item. Besides, the display should be consistent. By default, all <summary> should display the same way.

Comment 34

[Rod Smith]

A playground for <details><summary>
-----------------------------------

I've started a playground program to evaluate different aspects of <details><summary, that allows you to edit in wysiwyg and save the result to local disk.  There are just a few issues already outlined in a <details><summary> document there.

There are a few extra considerations in contentEditable mode, but mostly it's very very useful as it is.

Could any-one spare time to give it a try?  Follow the link to 

http://www.rodericksmith.plus.com/outlines/detailsSummary/

Feed-back welcomed, and information on whether the issues are bugs or my inadequate implementation!

Comment 35

[Rod Smith]

I have two key problems with copy/paste of contenteditable disclosure items, 
----------------------------------------------------------------------------
that web authors will experience.

-- can they be solved by the implementation?


Problem 1: Nested hidden item-trees are silently corrupted by copy/paste operations, 
  leading to unexpected and unseen corruption of the author's edits. (unnested is OK)
  
Problem 2: selection without encapsulating the disclosure (e.g. in a table element)
  is imprecise, leading to corruption e.g. by inserting a style tag between 
  details> and <summary while using a wysiwyg editor to style components  Could it
  show extent markers like 'table' does on selection?

They violate the 'principle of least surprise' and could generate negative comments. 

Is there some way around these issues, or can they be solved by modifying the implementation?
  (Could sizing handles appear on selection, like for the table element for problem 2?)

You can exercise Problem 1 by setting <body> to contenteditable and 
 copy-pasting the widget in the HTML code that follows:

--------------------------

<html ><head><meta charset=utf-8><title>DS issue: nested copy-paste</title></head>
<body  contenteditable="true" >

prior text0
<details><summary>summary0</summary>details0

prior text1
<details><summary>summary1</summary>details1</details>
following text1

</details>
following text0
</body></html>

--------------------------
More explanation under 
http://www.rodericksmith.plus.com/outlines/detailsSummary/issues/

Comment 36

[Cameron McCormack (:heycam)]

(In reply to Rod Smith from comment #35)
> Problem 1: Nested hidden item-trees are silently corrupted by copy/paste
> operations, 
>   leading to unexpected and unseen corruption of the author's edits.
> (unnested is OK)

I am not terribly familiar with the behaviour that the editor should have with regards to new elements like this.  I'll look into it.

> Problem 2: selection without encapsulating the disclosure (e.g. in a table
> element)
>   is imprecise, leading to corruption e.g. by inserting a style tag between 
>   details> and <summary while using a wysiwyg editor to style components 
> Could it
>   show extent markers like 'table' does on selection?

I don't understand this, sorry.  Do you have a problem selecting content within the <details> or its <summary>?

Comment 37

[Cameron McCormack (:heycam)]

Attachment: WIP (v0.3)

Another WIP patch.  Now we have native rendering of the disclosure widget on Mac, Windows and Linux.  (On Mac and Linux it's the same as a treetwisty, but on Windows it uses a more specific "expando" button.)  This is done with a -moz-appearance: disclosure-{closed,open} on the bullet frame for the <summary> element (which still is required to remain display:list-item for the moment).

On other platforms, and if you turn the appearance off with

  summary::-moz-list-bullet { -moz-appearance: none; }

you get the hand draw triangles from the previous patch.

Interaction is a little better -- clicking on the disclosure widget now does not select the <summary>'s contents.  However, the state of the widget (hovered, active) is the same as the <summary> element itself.  So if you hover or click on the contents of the <summary>, the widget will change appearance.  This is fine for Windows, at least, where this is the expected behaviour.  But it would be nice to only respond to clicks on the triangle on Mac, where this is the native behaviour you'd expect.  It's a bit tricky while I'm still using a bullet frame and don't have any content for the widget that can have separate state from the <summary> itself.

New builds:

Windows: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-7b7ab8532c80/try-win32/firefox-20.0a1.en-US.win32.zip
Linux: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-7b7ab8532c80/try-linux/firefox-20.0a1.en-US.linux-i686.tar.bz2
Mac: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-7b7ab8532c80/try-macosx64/firefox-20.0a1.en-US.mac.dmg

Comment 38

[Cameron McCormack (:heycam)]

Attachment: test

Here's a small example of customising the appearance of the disclosure widget.

Comment 39

[Cameron McCormack (:heycam)]

By the way, the native disclosure widgets are fixed size (on Windows they scale horribly, on Mac the triangle doesn't grow, just the padding around it does).  So it makes for slightly unexpected behaviour when you zoom the page in and out.

Comment 40

[Rod Smith]

(In reply to Cameron McCormack (:heycam) from comment #36)
> (In reply to Rod Smith from comment #35)
> > Problem 1: Nested hidden item-trees are silently corrupted by copy/paste
> > operations, 
> >   leading to unexpected and unseen corruption of the author's edits.
> > (unnested is OK)
> 
> I am not terribly familiar with the behaviour that the editor should have
> with regards to new elements like this.  I'll look into it.
> 

------------------------------------

When copy/pasting nested disclosure items, it is much better, from a web page editor's
standpoint, to preserve the nested disclosure items, their state and and their contents,
even when they are 'hidden'.  Otherwise, the whole 'nest' has to be re-typed.

I hope you may be able to make that happen . . .  thanks!

-------------------------------------


> > Problem 2: selection without encapsulating the disclosure (e.g. in a table
> > element)
> >   is imprecise, leading to corruption e.g. by inserting a style tag between 
> >   details> and <summary while using a wysiwyg editor to style components 
> > Could it
> >   show extent markers like 'table' does on selection?
> 
> I don't understand this, sorry.  Do you have a problem selecting content
> within the <details> or its <summary>?

----------------------------------

I can select content OK, but the boundary of the selected region sometimes does and other
times doesn't include the <details> or <summary> tag.  For example, if the details text is
selected in an editor add-on, and the 'make bold' button is pressed, the '<b>' tag sometimes
appears between the summary> and <details> tags in the middle of the disclosure item, and
other times (where it ought to be) after the <details> tag at the start of the details text.

The former causes some corruption of the disclosure item.  

It can be worked-around by enclosing the summary text content and (separately) the details 
text content in a table cell element -- the selection then snaps to the right place.  So it
would be nice to have a fix, but not essential.

I'll put up an example tomorrow (I'm out today).

Many thanks for investigating . . .
------------------------------------

Comment 41

[Mounir Lamouri (:mounir)]

(In reply to Cameron McCormack (:heycam) from comment #39)
> By the way, the native disclosure widgets are fixed size (on Windows they
> scale horribly, on Mac the triangle doesn't grow, just the padding around it
> does).  So it makes for slightly unexpected behaviour when you zoom the page
> in and out.

What do you mean by fixed size? They have a fixed width/height even if the author asked for something else? I feel like we usually don't do that. At least, for form elements, on Mac, we have the same problem (there are usually 2 or 3 sizes for each form control on MacOS) but instead of having a fixed size, we scale to the desired size. We should definitely use a default rendering (non-native) when the size would be too ugly on the platform, like a very huge button, but we don't do that yet.

IMO, we should make sure the default size renders well and whether scale or fallback to a default rendering for other sizes.

Comment 42

[Cameron McCormack (:heycam)]

(In reply to Mounir Lamouri (:mounir) from comment #41)
> What do you mean by fixed size? They have a fixed width/height even if the
> author asked for something else?

Yes, and also the fixed size is not relative to the font size.  Still for the author to request a different size, they would have to write:

  summary::-moz-list-bullet { width: 64px; height: 64px; }

for example.

> I feel like we usually don't do that. At
> least, for form elements, on Mac, we have the same problem (there are
> usually 2 or 3 sizes for each form control on MacOS) but instead of having a
> fixed size, we scale to the desired size. We should definitely use a default
> rendering (non-native) when the size would be too ugly on the platform, like
> a very huge button, but we don't do that yet.
> 
> IMO, we should make sure the default size renders well and whether scale or
> fallback to a default rendering for other sizes.

On Mac, I don't think I've ever seen a disclosure triangle rendered larger than the default size.  It might look OK if we scale before rendering it, but I wonder whether we should be doing that given that it would never appear larger than its usual size in native applications.  At least for Windows it seems to be painting a bitmap, so it's not going to look good at any size other than its usual one.

I guess the question then is how to decide when to fall back to the non-themed rendering.  For example, should this:

  <details>
    <summary style="font-size: 48px">summary</summary>
    details
  </details>

?  The native disclosure widget will look small next to the text.  We could decide for example that if the height of the text is greater than twice the height of the disclosure widget, then we switch to the non-themed rendering.

Comment 43

[Rod Smith]

(In reply to Rod Smith from comment #40)
> (In reply to Cameron McCormack (:heycam) from comment #36)
> > (In reply to Rod Smith from comment #35)

. . . . .



As promised, I've posted an 'active' example of the selection issue with an explanation at:

http://www.rodericksmith.plus.com/outlines/detailsSummary/issues/disclosureSelectionIssue.html

Comment 44

[Cameron McCormack (:heycam)]

Thanks for the explanation, Rod.  It looks like the editor needs to be taught that summary (and details too I imagine) are block elements and so inline formatting elements shouldn't go around them.

Comment 45

[sjw]

Uhm… How can I add this patch?

Comment 46

[Cameron McCormack (:heycam)]

(In reply to sjw from comment #45)
> Uhm… How can I add this patch?

If you are compiling your own Firefox builds, then you can apply the patch with:

  $ patch -p1 < thepatchfile

at the command line.  If not, you can try the builds from comment 37 to experiment with the <details> support so far.

Comment 47

[sjw]

(In reply to Cameron McCormack (:heycam) from comment #46)
> (In reply to sjw from comment #45)
> > Uhm… How can I add this patch?
> 
> If you are compiling your own Firefox builds, then you can apply the patch
> with:
> 
>   $ patch -p1 < thepatchfile
> 
> at the command line.  If not, you can try the builds from comment 37 to
> experiment with the <details> support so far.

Thanks!

Comment 48

[sjw]

Attachment: Possible bugs in the <details> and <summary> patch v0.3

Comment 49

[Mounir Lamouri (:mounir)]

(In reply to Cameron McCormack (:heycam) from comment #42)
> > I feel like we usually don't do that. At
> > least, for form elements, on Mac, we have the same problem (there are
> > usually 2 or 3 sizes for each form control on MacOS) but instead of having a
> > fixed size, we scale to the desired size. We should definitely use a default
> > rendering (non-native) when the size would be too ugly on the platform, like
> > a very huge button, but we don't do that yet.
> > 
> > IMO, we should make sure the default size renders well and whether scale or
> > fallback to a default rendering for other sizes.
> 
> On Mac, I don't think I've ever seen a disclosure triangle rendered larger
> than the default size.  It might look OK if we scale before rendering it,
> but I wonder whether we should be doing that given that it would never
> appear larger than its usual size in native applications.

It is exactly what happens for <progress>, <meter>, <input> and <button>: they all have at least one dimension restricted on MacOS X but we allow them to be rendered at any size. The output is sometimes ugly but we should fix that by falling back to the non-native rendering.

> At least for
> Windows it seems to be painting a bitmap, so it's not going to look good at
> any size other than its usual one.

Then maybe we should fallback to the non-native rendering as soon as we try to paint it to the non-default size?

> I guess the question then is how to decide when to fall back to the
> non-themed rendering.  For example, should this:
> 
>   <details>
>     <summary style="font-size: 48px">summary</summary>
>     details
>   </details>
> 
> ?  The native disclosure widget will look small next to the text.  We could
> decide for example that if the height of the text is greater than twice the
> height of the disclosure widget, then we switch to the non-themed rendering.

This rule or any other. We should prevent text with huge fonts to have a very small disclosure triangle because we only support one size.

Comment 50

[Cameron McCormack (:heycam)]

(In reply to sjw from comment #48)
> Created attachment 698429 [details]
> Possible bugs in the <details> and <summary> patch v0.3

Thanks for the comemnts, sjw.

For #1, you are right, the special styling should not be applied to the <summary> within the <buggy> element.

For #2 and #3 (which are the same?), I think it has the right behaviour.  An empty value is a valid way to specify a boolean attribute like open="".

For #4, something funny is going on there with frame construction...

For #5, I don't think that is incorrect behaviour, or anything specific to details/summary.  For example you could get the same word wrapping with deeply nested <ul><li> elements: http://mcc.id.au/temp/ul.html

Comment 51

[Cameron McCormack (:heycam)]

Attachment: WIP (v0.4)

A small update.  This should fix the editor problem from comment 43.  I also tweaked the shape of the disclosure triangle in the non-native rendering to more closely match the native Mac one, which I think looks nicer.

Linux: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-a01f355e991b/try-linux/firefox-20.0a1.en-US.linux-i686.tar.bz2
Mac: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-a01f355e991b/try-macosx64/firefox-20.0a1.en-US.mac.dmg
Windows: http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/[e-mail]-a01f355e991b/try-win32/firefox-20.0a1.en-US.win32.zip

Comment 52

[sjw]

>
> For #2 and #3 (which are the same?), I think it has the right behaviour.  An
> empty value is a valid way to specify a boolean attribute like open="".
> 
Thanks for your feedback.

#2 & #3: I don't know why, but there was missed something...

#2 the attribut shoud be the following: open=" ". My english is verry bad, but I read the W3C working draft and I think this means, that if the value is empty it schould be hidden.

"When the element is created, if the attribute is absent, the details should be hidden; if the attribute is absent, the details should be shown. Subsequently, if the attribute is removed, then the details should be hidden; if the attribute is added, the details should be shown."
http://www.w3.org/TR/2011/WD-html5-20110525/interactive-elements.html#the-details-element

#3 the attribut shoud be the following: open="buggy". This is a invalid value, so the details should be hidden.

Comment 53

[sjw]

Comment on attachment 698429 [details]
Possible bugs in the <details> and <summary> patch v0.3

><!DOCTYPE html>
><html>
><head>
><meta charset="utf-8">
><title>Details &amp; Summary Bugs (v0.3)</title>
></head>
><body>
>
><h1>Bug 1</h1>
><details>
><summary>Summary</summary>
><p>Details</p>
><buggy><!--invalid tag name-->
><summary>Summary 2</summary>
><p>Invalid tag name. This should be ignored.</p>
></buggy>
></details>
><br>
><textarea cols="60" rows="10">
><h1>Bug 1</h1>
><details>
><summary>Summary</summary>
><p>Details</p>
><buggy><!--invalid tag name-->
><summary>Summary 2</summary>
><p>Invalid tag name. This should be ignored.</p>
></buggy>
></details>
></textarea>
><br><br>
><h1>Bug 2</h1>
><details open="buggy"><!--invalid attribute value-->
><summary>Summary</summary>
><p>Invalid attribute value. This should be ignored.</p>
></details>
><br>
><textarea cols="60" rows="10">
><h1>Bug 2</h1>
><details open="buggy"><!--invalid attribute value-->
><summary>Summary</summary>
><p>Invalid attribute value. This should be ignored.</p>
></details>
></textarea>
><br><br>
><h1>Bug 3</h1>
><details open=" "><!--should be closed by default-->
><summary>Summary</summary>
><p>I think this should be closed by default.</p>
></details>
><br>
><textarea cols="60" rows="10">
><h1>Bug 3</h1>
><details open><!--should be closed by default-->
><summary>Summary</summary>
><p>I think this should be closed by default.</p>
></details>
></textarea>
><br><br>
><h1>Bug 4</h1>
><details>
><p>This one have two summary tags.</p>
><summary>Summary 1.1</summary>
><summary>Summary 1.2</summary><!--seccond summary tag-->
><details>
><summary>Summary 2</summary>
><p>So this summary tag have some troubles on first useing (it change its position).</p>
></details>
></details>
><br>
><textarea cols="60" rows="10">
><h1>Bug 4</h1>
><details>
><p>This one have two summary tags.</p>
><summary>Summary 1.1</summary>
><summary>Summary 1.2</summary><!--seccond summary tag-->
><details>
><summary>Summary 2</summary>
><p>So this summary tag have some troubles on first useing (it change its position).</p>
></details>
></details>
></textarea>
><br><br>
><h1>Bug 5</h1><!--Line Breaks-->
><details>
><summary>Line Break</summary>
><p>The browser makes a line break on every word (sroll down to see what I mean), but it is only necessary in the last open one.</p>
><details>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
><br>
><textarea cols="60" rows="10">
><h1>Bug 5</h1><!--Line Breaks-->
><details>
><summary>Line Break</summary>
><p>The browser makes a line break on every word (sroll down to see what I mean), but it is only necessary in the last open one.</p>
><details>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
><details open>
><summary>Summary Summary</summary>
><p>Details Details</p>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></details>
></textarea>
></body>
></html>

Comment 54

[sjw]

Attachment: Possible bugs in the <details> and <summary> patch v0.4

Comment 55

[sjw]

Attachment: Possible bugs in the <details> and <summary> patch v0.4.1

minor bug fixing

Comment 56

[sjw]

Attachment: Possible bugs in the <details> and <summary> patch v0.4.2

Added two more bugs ("tabindex-bug" & "editable-bug").

Comment 57

[Cameron McCormack (:heycam)]

(In reply to sjw from comment #52)
> #2 & #3: I don't know why, but there was missed something...
> 
> #2 the attribut shoud be the following: open=" ". My english is verry bad,
> but I read the W3C working draft and I think this means, that if the value
> is empty it schould be hidden.

No, if the attribute is present -- regardless of whether it has a non-empty value -- then the <details> element should be open.  This is the way boolean attributes in HTML work:

http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#boolean-attributes

Comment 58

[sjw]

Attachment: Possible bugs in the <details> and <summary> patch v0.4.3

Comment 59

[sjw]

(In reply to Cameron McCormack (:heycam) from comment #57)
> No, if the attribute is present -- regardless of whether it has a non-empty
> value -- then the <details> element should be open.  This is the way boolean
> attributes in HTML work:
> 
> http://www.whatwg.org/specs/web-apps/current-work/multipage/common-
> microsyntaxes.html#boolean-attributes


Okay, sorry about that. I hope the other bugs are a real help.

Comment 60

[wind]

Great work everyone!

Comment 61

[Florian Bender]

Any news on this one?

Comment 62

[Cameron McCormack (:heycam)]

I should be getting back to it soon; other work has priority at the moment.

Comment 63

[Rod Smith]

Does anyone know when the <details><summary> capabilities in the snapshots above can be incorporated into the mainstream Firefox releases (so I can provide my outlined manuals for anyone to use)?

(Having to use a development snapshot for a long period causes me problems because some updated plug-ins are no longer compatible)

Thanks

Comment 64

[Cameron McCormack (:heycam)]

Other work has still got in front of this work, sorry.  Current thing ahead of this bug for me are bug 773296 and bug 731271.

Comment 65

[Cameron McCormack (:heycam)]

Attachment: WIP (v0.5)

Rebased.

Comment 67

[aja+bugzilla]

FYI, in case you haven't noticed it yet: http://html5.org/r/8251

Hixie: aja, heycam|away: let me know if you have feedback on that, i didn't have much to go on despite asking around :-)
Hixie: just that several people wanted it
Hixie: and that there was no good way to do it before
Hixie: (short of mutation observers)

Comment 68

[crazytonyi]

I'm relieved to see that this has been actively developed and isn't one of those "keep the ball in the air" type feature requests where a developer is always working on something "a little more important" etc. A few thoughts:

1. The issue with form validation -- is this an issue with native form validation or custom validators? If it's a custom validator, I think the burden of expanding the relevant summary elements should be on the developer of the custom code, since they will know which non-visible fields require visibility and since the additional code to do this should be trivial (basically "for all invalid input ancestors of type summary with attribute "open" set to false, set to true).  However if it's native validation (such as an input of type "email" with invalid data or an empty input with "required" attribute) would it make sense to trigger the appropriate event (I think it would be 'click' for the "options" element) on the non-valid input and set the event to bubble? If you think of the "click" event (or "touch", or equivalent input-device-neutral event) as "poke", it would make sense that each invalid form input would get poked on submit and that if it bubbled up, this would mean any nested "summary" elements would open, as well as any custom interactive scripting.

The only downside to the bubbling idea is that the "invalid poke" event + open attribute value would need to be logically OR so that an already expanded summary element would not close.

Second:

Again, I am incredibly impressed with the amount of thought that has gone into this feature. Much more than I would have initially though (form data being corrupted, nested summaries, block vs list item, etc). But at the risk of sounding pushy, this feature needs to go out the front door ASAP. Reasons for increased priority:

http://www.w3.org/TR/html5/ -- The following features are at risk and may be removed due to lack of implementation. ( summary and details, high on the list).

When this bug was submitted 2 years ago, no layout engines had implemented this part of the spec. Now, there are only two  :

http://html5test.com/compare/feature/elements-interactive-details.html
http://caniuse.com/#feat=details

History has proven time and again that if IE drags its heels but the other "warriors" have adopted a drafted feature, the feature will survive to final recommendation, but if IE + another major browser lack the feature, it will circle the drain with only techno-political chances of survival.  If this feature isn't rolled out in a public release of Gecko soon, bumping the browser adoption percentage over 70 and getting true adoption confidence started, not only will this very useful and lightweight feature wither before it had a chance in the wild, the 2 years of effort above will have been for nothing.

I, for one, would really like to see this feature stick around, for no other reason than it's already in place conceptually on so many sites (wikipedia's mobile site being a big example) so it clearly fills a need and is not just  a neat idea. (Side note, I will make similar passionate pleas for features that are at this point just a neat idea as well: https://www.w3.org/Bugs/Public/show_bug.cgi?id=23589 )

Comment 69

[Rod Smith]

I'll second the call to implement <details><summary> quickly in the mainstream Mozilla releases.

It makes outliner and structured website features almost pure and simple HTML5.

Otherwise the web pages must be cluttered with JavaScript that distracts the uninitiated from understanding how the  page works.

Comment 70

[adam.taylor]

I agree. This will allow us to begin to replace hundreds of 'disclosure' widgets across the web; it's one of the most common patterns in use, and browsers really ought to allow it natively.

Firefox support along with Chrome will give it some real momentum.

Comment 71

[sjw]

Since Blink Opera supports it, too. The whiteboard maight be updated.

Comment 72

[Kyli]

Confirmed works in Opera as of right now

Comment 73

[Florian Bender]

Please don't add non-constructive "me too" comments, they will not influence the speed of development at all (at least not positively). Only comment on this bug if you have patches for implementation or tests that add value.

Comment 74

[Mike Gifford]

<details> and <summary> have been adopted as part of Drupal 8, so I do really hope that this gets brought into FF.  Florian Bender, I do appreciate the "me too" concerns, but it's a significant piece of the Internet at the moment.

Comment 75

[Andy Earnshaw]

One thing that Chrome doesn't do and might be worth considering is when an expanding <details> box is rendered beyond the current scroll position, no scrolling occurs to bring the newly rendered content into view.  At the moment, this means I'm overriding Chrome's behaviour altogether in order to give the user the best experience.

Having to scroll manually is more a minor annoyance than anything else, but it would be useful if the bottom of the box is aligned with the bottom of the viewport (in the case of vertical scrolling), but only if the box is smaller than the vertical height of the document.

Comment 76

[Matt Basta [:basta]]

(In reply to Andy Earnshaw from comment #75)
> One thing that Chrome doesn't do and might be worth considering is when an
> expanding <details> box is rendered beyond the current scroll position, no
> scrolling occurs to bring the newly rendered content into view.  At the
> moment, this means I'm overriding Chrome's behaviour altogether in order to
> give the user the best experience.
> 
> Having to scroll manually is more a minor annoyance than anything else, but
> it would be useful if the bottom of the box is aligned with the bottom of
> the viewport (in the case of vertical scrolling), but only if the box is
> smaller than the vertical height of the document.

This is well outside the scope of this ticket. The user can already perform that scrolling using JS, and there may be cases where the developer does not intend for the browser to scroll.

Comment 77

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I agree with Matt. I often find myself wanting to expand a consecutive subset of members in a list, and in Chrome, this is easily done by repeatedly pushing the adjacent arrows starting at the *bottom* of the subset so that the adjacent arrows don't disappear or move under my finger/mouse cursor. Auto-scrolling on every expand would severely hamper this use-case.

Comment 78

[Andy Earnshaw]

(In reply to Matt Basta [:basta] from comment #76)
> This is well outside the scope of this ticket. 
I don't know if I'd agree that it's "well outside".  Borderline, perhaps, but it seems like something that would make sense to add at the time of implementation rather than waiting until it's finished and then asking someone to go back and make the changes then.  Changing behaviour after implementation might cause confusion among developers and users alike.

> The user can already perform that scrolling using JS
Can you elaborate on this?  Do you mean user of the HTML elements or end user of the browser?  I often take "user" to mean the latter, in which case the majority of users are unable to write their own JS and not everyone has JS enabled for every site.

> and there may be cases where the developer does not intend for the browser to scroll.
For which there is .addEventListener('click', ...) and .preventDefault().

(In reply to Jan-Ivar Bruaroey [:jib] from comment #77)
> I agree with Matt. I often find myself wanting to expand a consecutive
> subset of members in a list, and in Chrome, this is easily done by
> repeatedly pushing the adjacent arrows starting at the *bottom* of the
> subset so that the adjacent arrows don't disappear or move under my
> finger/mouse cursor. Auto-scrolling on every expand would severely hamper
> this use-case.
I understand, but surely the majority use case in this situation would be expanding a <details> block to read immediately?  Shouldn't we be aiming to address the majority use case?

Perhaps a modifier key while clicking could address the use case where scrolling isn't desired.

Comment 79

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(In reply to Andy Earnshaw from comment #78)
> Changing behaviour after implementation might cause confusion among developers and users alike.

The same argument can be used against implementing behavior different from Chrome here.

Comment 80

[Andy Earnshaw]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #79)
> (In reply to Andy Earnshaw from comment #78)
> > Changing behaviour after implementation might cause confusion among developers and users alike.
> 
> The same argument can be used against implementing behavior different from
> Chrome here.
I don't think vendor implementation and developer implementation is broad enough right now for many end users to even realise that WebKit/Blink's behaviour is built in.  In my case I need to also scroll the element into view, so I'm overriding WebKit's behaviour with the same code I'm using to polyfill other browsers (since it requires more code to detect and the implementation is fairly simple anyway).

Regardless, it was merely a suggestion and I've made my point.  I could argue usability all day, but it's not like this is a major breaking issue.

Comment 82

[Alice Wonder]

There are two places where I regularly use this tag:

A) On a 'works cited' page, I will have the author as the summary and the full bibtex style reference in the rest of the details. I have JS that automatically open the node if the page was access by a link with a # pointing to specific work.

Having the works cited in a details node makes it easy for someone to scan for the work they are looking for and open that node.

The other place I use it - in articles where I have article / section structure, my php auto-generates a table of contents that goes two sections deep.

That ToC is put into a nav div as a child of the details node, and the summary tag says "Table of Contents" - the details is inserted under the h1 header at beginning of article.

Most people visiting the page probably don't need the ToC - but if a visitor is looking for something specific they know is in the page, the ToC helps them find it.

This works beautifully in both Chrome and Midori. It would be nice if it worked in FireFox too. I've been waiting YEARS for it to work in FireFox, I was an early adopter of the tag.

I wrote JavaScript to emulate it for browsers w/o support, but the JavaScript broke with a jQuery update. Rather than fix it, I just removed it. I shouldn't have to use JavaScript to provide what HTML5 browsers should already do.

Comment 83

[crazytonyi]

(In reply to Alice Wonder from comment #82)
> There are two places where I regularly use this tag:
> 
> A) On a 'works cited' page, I will have the author as the summary and the
> full bibtex style reference in the rest of the details. I have JS that
> automatically open the node if the page was access by a link with a #
> pointing to specific work.
> 
> Having the works cited in a details node makes it easy for someone to scan
> for the work they are looking for and open that node.
> 
> The other place I use it - in articles where I have article / section
> structure, my php auto-generates a table of contents that goes two sections
> deep.
> 
> That ToC is put into a nav div as a child of the details node, and the
> summary tag says "Table of Contents" - the details is inserted under the h1
> header at beginning of article.
> 
> Most people visiting the page probably don't need the ToC - but if a visitor
> is looking for something specific they know is in the page, the ToC helps
> them find it.
> 
> This works beautifully in both Chrome and Midori. It would be nice if it
> worked in FireFox too. I've been waiting YEARS for it to work in FireFox, I
> was an early adopter of the tag.
> 
> I wrote JavaScript to emulate it for browsers w/o support, but the
> JavaScript broke with a jQuery update. Rather than fix it, I just removed
> it. I shouldn't have to use JavaScript to provide what HTML5 browsers should
> already do.

I know we're supposed to limit chatter on bug comments, but Alice's last comment made me think of a specific aspect of details element I was curious about. It may be easy to find in spec, but more interesting to ask here. Is browser's 'Find' feature expected to expand results found in folded/collapsed detail elements, so that in Alice's use case of bibliographic citations, if a user searches for 'Princeton', would results be visible if in the full citation that's collapsed? And if yes, should it expand only when the user uses the Find 'next/back' option (and the 'next' result is inside collapsed section) and then the details folds back up when they move on to results outside of that element? Or should all folded elements expand based on Find results and stay expanded while Find is active? Even further, in any of these scenarios, should any/all details that were expanded due to Find go back to pre-Find state when Find is closed?

On the one hand, it may be a bit sloppy if searching for 'smith' results in all details being expanded and requires page reload (or manually collapsing each) to get them out of the way. On the other hand, if I spot my match (yay!) and then close Find, expecting to search further or do some follow up task, I'd be pretty annoyed when the results ran away from me, mocking me.

Comment 84

[Rod Smith]

(In reply to Alice Wonder from comment #82)
. . . .
> 
> I wrote JavaScript to emulate it for browsers w/o support, but the
> JavaScript broke with a jQuery update. Rather than fix it, I just removed
> it. I shouldn't have to use JavaScript to provide what HTML5 browsers should
> already do.

You don't need jQuery to emulate <details><summary> -- you can adapt from several authors.
My take was to simplify from https://github.com/tyleruebele/details-shim as follows, plus a call to javascript and styling in the head of the HTML where the <details><summary> tags lie (you could in-line them if appropriate) :

(calls ...within the head section of the HTML page)


    <script type="text/javascript" src="wte.js"></script>
    <link rel="stylesheet" type="text/css" href="wte.css" > </link>

--------------------
(my file:wte.js -- for the control)
--------------------

/* Simplified from: Copyright (c) 2006-2013 Tyler Uebele * Released under the MIT license. * latest at https://github.com/tyleruebele/details-shim * minified by Google Closure Compiler */

function details_shim(a) {
  if (!(a && 'nodeType' in a && 'tagName' in a)) return details_shim.init();
  var b;
  if ('details' == a.tagName.toLowerCase()) b = a.getElementsByTagName('summary') [0];
   else if (a.parentNode && 'summary' == a.tagName.toLowerCase()) b = a,
  a = b.parentNode;
   else return !1;
  if ('boolean' == typeof a.open) return !1;
  var c = a.outerHTML || (new XMLSerializer).serializeToString(a),
  c = c.substring(0, c.indexOf('>')),
  c = - 1 != c.indexOf('open') && - 1 == c.indexOf('open=""') ? 'open' : 'closed';
  a.setAttribute('data-open', c);
  a.className +=
  ' ' + c;
  b.addEventListener('click', function () {
    details_shim.toggle(a)
  });
  Object.defineProperty(a, 'open', {
    get: function () {
      return 'open' == this.getAttribute('data-open')
    },
    set: function (a) {
      details_shim.toggle(this, a)
    }
  });
  for (b = 0; b < a.childNodes.length; b++) if (3 == a.childNodes[b].nodeType && /[^\s]/.test(a.childNodes[b].data)) {
    var c = document.createElement('span'),
    d = a.childNodes[b];
    a.insertBefore(c, d);
    a.removeChild(d);
    c.appendChild(d)
  }
}
details_shim.toggle = function (a, b) {
  b = 'undefined' === typeof b ? 'open' == a.getAttribute('data-open') ? 'closed' : 'open' : b ? 'open' : 'closed';
  a.setAttribute('data-open', b);
  a.className = a.className.replace(/\bopen\b|\bclosed\b/g, ' ') + ' ' + b
};
details_shim.init = function () {
  for (var a = document.getElementsByTagName('summary'), b = 0; b < a.length; b++) details_shim(a[b])
};
window.addEventListener ? window.addEventListener('load', details_shim.init, !1)  : window.attachEvent && window.attachEvent('onload', details_shim.init);

----------

(and file wte.css for the styling -- your styling requirement may vary)

----------
details,summary{display:block}details.closed>*{display:none}
details.closed>summary{display:block}
details.open>summary{display:block}
details.closed>summary:before{display:inline-block;content:"\25b6";padding:0 .1em;margin-right:.4em;font-size:.9em}
details.open>summary:before{display:inline-block;content:"\25bc";padding:0;margin-right:.35em}
details {padding-left: 1em}
summary {position: relative; left: -1em; text-decoration: underline; font-weight:bold;
  color: blue;
}

-----

This is in regular use with many outlines including ~500 chapters of text-books  -- google ' learningbyoutlines ' (all one word) or http://www.rodericksmith.plus.com for details.

Comment 85

[Sebastian Zartner [:sebo]]

> Is browser's 'Find' feature expected to expand results found in folded/collapsed 
> detail elements, so that in Alice's use case of bibliographic citations, if a 
> user searches for 'Princeton', would  results be visible if in the full citation 
> that's collapsed?

That's out of the scope of this issue. A better place to ask this might be the mozilla.dev.tech.layout discussion group[1].

Sebastian

[1] https://groups.google.com/forum/#!forum/mozilla.dev.tech.layout

Comment 86

[wig_2006]

I have HTML5's <details> and <summary> as same , because have HTML5's <details> and <summary> since Google Chrome 12.

Comment 87

[wig_2006]

(In reply to wig_2006 from comment #86)
> I have HTML5's <details> and <summary> as same , because have HTML5's
> <details> and <summary> since Google Chrome 12.

Add technicalvalue

http://s29.postimg.org/vwtmpiy5h/Screenshot_from_2015_04_30_20_56_43.png 
- Firefox HTML5 test to not support HTML5's <details> and <summary> (technicalvalue from html5test.com).

http://s29.postimg.org/wauyp4i91/Screenshot_from_2015_04_30_20_58_03.png
http://s29.postimg.org/xr6h79l5x/Screenshot_from_2015_04_30_20_58_16.png
- Compare HTML5's <details> and <summary> between Firefox and Chrome (technicalvalue from caniuse.com).

Comment 88

[Sebastian Zartner [:sebo]]

(In reply to wig_2006 from comment #87)
> (In reply to wig_2006 from comment #86)
> > I have HTML5's <details> and <summary> as same , because have HTML5's
> > <details> and <summary> since Google Chrome 12.
> 
> Add technicalvalue

The tag 'notechnicalvalue' means that the comment didn't add information to implement this feature. Your second comment also just tells that Firefox doesn't have this implemented yet, which is clear, because otherwise this bug would already be closed. :-)
Also that Chrome already has this implemented is tracked by the whiteboard info [parity-blink].

Sebastian

Comment 89

[Rich Schwerdtfeger]

The digital publishing industry is looking at <details><summary> as a vehicle to embed image descriptions which would include the ability to reference an external document with an embedded iFrame to allow for crowd sourcing of image descriptions or provisions for alternative content such as special plugins for charts. One of the proposals has been for an aria-describedat feature that points to another URL but a details/summary option tied to figure and figcaption would be better as other users besides assistive technologies would benefit. 

This would be used in EPUB authoring tools.

Comment 90

[secretgspot]

how embarrassing, 2015 and details/summary has not been implemented?!?! How hard is it to implement such simple component?  After this project uninstalling Firefox and no longer offer it in the product list.  Will only be supporting Chrome all the way.   Chrome had it since Chrome 12, it's on Chrome 45 now. 

Seriously, how embarrassing!

Comment 91

[Sylvestre Ledru [:Sylvestre]]

Cameron, do you still have plans to work on this? If not, maybe reset the assignee? Thanks

Comment 92

[Cameron McCormack (:heycam)]

No, but I spoke to TYLin the other week who said he might be interested in finishing it off?

Comment 93

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Yes. I'm interested in bring your patch to the finish line.

Comment 94

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: WIP branch on GitHub

My WIP branch on Github. It is not ready to be tested yet.

Comment 95

[Mike Gifford]

Drupal 8 Core is launching tomorrow. It comes with summary/details built-in. People are obviously already building sites with Drupal 8, but there will be a lot more in the coming months. 

Would be great to improve the support in this great browser.

Comment 96

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Teach parser about <details> and <summary>

Bug 591737 - Teach parser about <details> and <summary>

Comment 97

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils

Bug 591737 - Add details and summary to nsHTMLEditUtils

Comment 98

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

Bug 591737 - Add HTMLDetailsElement and webidl interface

Comment 99

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add HTMLSummaryElement

Bug 591737 - Add HTMLSummaryElement

Comment 100

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add SummaryFrame

Bug 591737 - Make nsIFrame::PrincipalChildList a const function

Since nsIFrame::GetChildList() is a const function, PrincipalChildList()
should be a const function as well.

Comment 101

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add DetailsFrame

Bug 591737 - Add DetailsFrame

Comment 102

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

Bug 591737 - Add SummaryFrame

Comment 103

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

Bug 591737 - Construct details and summary in nsCSSFrameConstructor

For now, we use "display: list-item" on main summary element to draw the
the disclosure widget. We might need a way to customize the disclosure
widget later.

Comment 104

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Provide a default summary element by DetailsFrame

Bug 591737 - Provide a default summary element by DetailsFrame

If a <details> lacks a direct <summary> child, we need to construct a
default one.

Comment 105

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add reftests for details and summary

Bug 591737 - Add reftests for details and summary

Comment 106

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add crashtest for details and summary

Bug 591737 - Add crashtest for details and summary

Comment 107

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Implement toggling open details by mouse click

Bug 591737 - Implement toggling open details by mouse click

Comment 108

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

Bug 591737 - Avoid dispatch mouse double click to content not in doc

When double-clicking on a default anonymous <summary> element, the first
eMouseClick will make the summary element remove from the document. This
generates an assert in PresShell::HandleEventWithTarget().

To prevent this assert, ensure the mouseContent is in document before
dispatching the eMouseDoubleClick event.

Since GetCrossShadowCurrentDoc() was deprecated, replace it with
GetComposedDoc().

Comment 109

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689422 [details]
MozReview Request: Bug 591737 - Teach parser about <details> and <summary>

Bug 591737 - Teach parser about <details> and <summary>

Comment 110

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689423 [details]
MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils

Bug 591737 - Add details and summary to nsHTMLEditUtils

Comment 111

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

Bug 591737 - Add HTMLDetailsElement and webidl interface

Comment 112

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689425 [details]
MozReview Request: Bug 591737 - Add HTMLSummaryElement

Bug 591737 - Add HTMLSummaryElement

Comment 113

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689426 [details]
MozReview Request: Bug 591737 - Add SummaryFrame

Bug 591737 - Make nsIFrame::PrincipalChildList a const function

Since nsIFrame::GetChildList() is a const function, PrincipalChildList()
should be a const function as well.

Comment 114

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689427 [details]
MozReview Request: Bug 591737 - Add DetailsFrame

Bug 591737 - Add DetailsFrame

Comment 115

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689428 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

Bug 591737 - Add SummaryFrame

Comment 116

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689429 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

Bug 591737 - Construct details and summary in nsCSSFrameConstructor

For now, we use "display: list-item" on main summary element to draw the
the disclosure widget. We might need a way to customize the disclosure
widget later.

Comment 117

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689430 [details]
MozReview Request: Bug 591737 - Provide a default summary element by DetailsFrame

Bug 591737 - Provide a default summary element by DetailsFrame

If a <details> lacks a direct <summary> child, we need to construct a
default one.

Comment 118

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689431 [details]
MozReview Request: Bug 591737 - Add reftests for details and summary

Bug 591737 - Add reftests for details and summary

Comment 119

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689432 [details]
MozReview Request: Bug 591737 - Add crashtest for details and summary

Bug 591737 - Add crashtest for details and summary

Comment 120

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689434 [details]
MozReview Request: Bug 591737 - Implement toggling open details by mouse click

Bug 591737 - Implement toggling open details by mouse click

Comment 121

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689435 [details]
MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

Bug 591737 - Avoid dispatch mouse double click to content not in doc

When double-clicking on a default anonymous <summary> element, the first
eMouseClick will make the summary element remove from the document. This
generates an assert in PresShell::HandleEventWithTarget().

To prevent this assert, ensure the mouseContent is in document before
dispatching the eMouseDoubleClick event.

Since GetCrossShadowCurrentDoc() was deprecated, replace it with
GetComposedDoc().

Comment 122

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Add reftest for mouse click on summary

Bug 591737 - Add reftest for mouse click on summary

Comment 123

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Fix test_HTMLSpec.html

Bug 591737 - Fix test_HTMLSpec.html

Comment 124

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: MozReview Request: Bug 591737 - Mark tests in web-platform-test pass

Bug 591737 - Mark tests in web-platform-test pass

Now we have <details> and <summary> tags implemented. Remove those lines
marked as "expected: FAIL" to let the tests pass.

Comment 125

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #97)
> Created attachment 8689423 [details]
> MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils
> 
> Bug 591737 - Add details and summary to nsHTMLEditUtils

I wonder, why did you request this review to me? I know there is no people working on editor as an owner nor a peer, so, if you don't find better people to review this, I'll try, though.

Comment 126

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25615/#review23043

::: dom/events/EventStateManager.cpp:4658
(Diff revision 1)
>          ret = presShell->HandleEventWithTarget(&event2, currentTarget,

HandleEventWithTarget() is called here.

Comment 127

[steve faulkner]

Is there a test build available?

Comment 128

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Re comment 125: 
Perhaps Ehsan still review editor code. I'll flag him for the review.

Comment 130

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Re comment 128: 

Latest try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b106cf76d43e

Mac: http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-b106cf76d43e7e377bda2909a6e9d7ce4f23403f/try-macosx64/firefox-45.0a1.en-US.mac.dmg
Linux: http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-b106cf76d43e7e377bda2909a6e9d7ce4f23403f/try-linux64/firefox-45.0a1.en-US.linux-x86_64.tar.bz2
Windows: http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-b106cf76d43e7e377bda2909a6e9d7ce4f23403f/try-win32/firefox-45.0a1.en-US.win32.zip

Comment 131

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #128)
> Re comment 125: 
> Perhaps Ehsan still review editor code. I'll flag him for the review.

If he is busy, feel free to ask me again.

Comment 132

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8689439 [details]
MozReview Request: Bug 591737 - Mark tests in web-platform-test pass

https://reviewboard.mozilla.org/r/25621/#review23053

This commit should be squashed into the commits that actually fix the tests.

Comment 133

[alexander :surkov (:asurkov)]

Comment on attachment 8689438 [details]
MozReview Request: Bug 591737 - Fix test_HTMLSpec.html

https://reviewboard.mozilla.org/r/25619/#review23055

r=me but I would prefer to have something like this. If it works. If it doesn't then we need a bug on it.

var obj = {
  role: ROLE_GROUPING,
  absentStates: STATE_EXPANDED,
  children: [
    { role: ROLE_LABEL }
  ]
};
testElm("details", obj);

obj = {
  role: ROLE_GROUPING,
  states: STATE_EXPANDED,
  children: [
    { role: ROLE_LABEL },
    { role: ROLE_ENTRY }
  ]
};
testElm("details_open", obj);
 
<details id="details">
  <summary><label for="name">Name:</label></summary>
  <input type="text" id="name" name="name" />
</details>

 <details id="details_open" open>
  <summary><label for="name">Name:</label></summary>
  <input type="text" id="name" name="name" />
</details>

Comment 134

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #133)
> Comment on attachment 8689438 [details]
> MozReview Request: Bug 591737 - Fix test_HTMLSpec.html
> 
> https://reviewboard.mozilla.org/r/25619/#review23055
> 
> r=me but I would prefer to have something like this. If it works. If it
> doesn't then we need a bug on it.

we do have bug 634004 for this

Comment 135

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

(my review isn't needed here, unless bz wants to move some of the reviews to me.)

Comment 136

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

https://reviewboard.mozilla.org/r/25595/#review23067

::: dom/html/HTMLDetailsElement.h:18
(Diff revision 1)
> +  using NodeInfo = mozilla::dom::NodeInfo;

Why do you need that?  You're already inside namespace mozilla::dom.

::: dom/html/HTMLDetailsElement.h:20
(Diff revision 1)
> +  explicit HTMLDetailsElement(already_AddRefed<NodeInfo>& aNodeInfo);

I think we've generally tried to make constructors like this inline for elements.

::: dom/html/HTMLDetailsElement.h:29
(Diff revision 1)
> +  void SetOpen(bool aOpen) { SetBoolAttr(nsGkAtoms::open, aOpen); }

This needs to be SetHTMLBoolAttr(nsGkAtoms:open, aOpen, aRv).  And you need to mark this attribute as [SetterThrows] in the IDL.

::: dom/html/nsGenericHTMLElement.h:1761
(Diff revision 1)
> +NS_DECLARE_NS_NEW_HTML_ELEMENT(Details)

Should come before the Div entry.

r=me with the above fixed.

Comment 137

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689425 [details]
MozReview Request: Bug 591737 - Add HTMLSummaryElement

https://reviewboard.mozilla.org/r/25597/#review23069

r=me

::: dom/html/HTMLSummaryElement.h:18
(Diff revision 1)
> +  using NodeInfo = mozilla::dom::NodeInfo;

Again, don't need that.

::: dom/html/HTMLSummaryElement.h:20
(Diff revision 1)
> +  explicit HTMLSummaryElement(already_AddRefed<NodeInfo>& aNodeInfo);

Inline.

Comment 138

[Boris Zbarsky [:bzbarsky]]

> Why do you need that?  You're already inside namespace mozilla::dom.

Ah, you need it because bareword NodeInfo resolves to nsINode::NodeInfo, right.  <sigh>.  Ignore that comment, for both files.

Comment 139

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689426 [details]
MozReview Request: Bug 591737 - Add SummaryFrame

https://reviewboard.mozilla.org/r/25599/#review23073

r=me

Comment 140

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8689422 [details]
MozReview Request: Bug 591737 - Teach parser about <details> and <summary>

er, sorry, you were right the first time, although I don't know why we'd need to modify the old parser

Comment 141

[Henri Sivonen (:hsivonen)]

(In reply to David Baron [:dbaron] ⌚UTC-8 from comment #140)
> Comment on attachment 8689422 [details]
> MozReview Request: Bug 591737 - Teach parser about <details> and <summary>
> 
> er, sorry, you were right the first time, although I don't know why we'd
> need to modify the old parser

nsHTMLTagList.h is still used for generating the factory machinery for making created element nodes have the right C++ class.

We should probably remove most of parser/htmlparser/nsHTMLTags.cpp, though.

(I'd r+ this.)

Comment 142

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689428 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

https://reviewboard.mozilla.org/r/25603/#review23085

r=me, I think.  I'm pretty rusty on frame classes...

Comment 143

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689427 [details]
MozReview Request: Bug 591737 - Add DetailsFrame

https://reviewboard.mozilla.org/r/25601/#review23075

::: layout/generic/DetailsFrame.h:15
(Diff revision 1)
> +// See nsCSSFrameConstructor::ConstructDetailsFrame for the structure of an

s/an/a/

::: layout/generic/DetailsFrame.cpp:82
(Diff revision 1)
> +    PresContext()->PresShell()->RecreateFramesFor(mContent);

Why are you handling this via AttributeChanged on the frame instead of just having your element implement GetAttributeChangeHint appropriately?  The latter would be far preferable, since it will coalesce much better with other style changes.

r=me with the above fixed, pending what I see in the frame constructor patch.

Comment 144

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689429 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

https://reviewboard.mozilla.org/r/25605/#review23087

This is a great start, but there are some issues.  No surprise; this part is complicated.  ;)

What should happen if the <details> is styled to be taken out of line (floated, absolutely positioned, fixed positioned)?  From a quick glance at Chrome's behavior and your code, the behavior you're going to end up with is quite different from Chrome's, right?

::: layout/base/nsCSSFrameConstructor.h:387
(Diff revision 1)
> +                                 FrameConstructionItemList& aItems);

Document what this argument is supposed to mean.  But see more on this below....

::: layout/base/nsCSSFrameConstructor.cpp:3236
(Diff revision 1)
> +nsCSSFrameConstructor::ConstructDetailsFrame(nsFrameConstructorState& aState,

I don't see how this handles values of "overflow" other than "visible" on <details>.

::: layout/base/nsCSSFrameConstructor.cpp:3245
(Diff revision 1)
> +  nsIContent* details = aItem.mContent;
> +  RefPtr<nsStyleContext> styleContext = aItem.mStyleContext;

Really best to make these const pointers as elsewhere (const pointers, not pointers to const, note).

::: layout/base/nsCSSFrameConstructor.cpp:3281
(Diff revision 1)
> +      if (f && f->GetType() == nsGkAtoms::summaryFrame) {

I expect you will want f->GetContentInsertionFrame()->GetType() here once you support overflow on <summary>.

::: layout/base/nsCSSFrameConstructor.cpp:3282
(Diff revision 1)
> +        // Take out the first summary frame and put it as a child of the

Have you tested what happens here with floats inside the <summary>?  I _think_ it should work because <summary> is forced to a block-like display so they get parented to it, not to the anonymous detailsContentFrame, but please make sure you have tests.

Also, I assume you have tests for display:inline styling on <details> and the like, right?  If you don't, please add them.

Have you tested what happens with:

  <details style="height: 200px">
    <summary style="height: 100%">Stuff</summary>
  </details>

as well as:

  <details style="height: 200px">
    <div style="height: 100%">Stuff</summary>
  </details>

?  I see nothing in the reftests testing this sort of thing, and while the spec here is useless I would expect that the 100% height nodes in those testcases would end up with a 200px height.

Have you tested how this affects z-ordering of absolutely positioned elements, where one is a child of the <summary> and one is a child of the <details> that comes before <summary> in the DOM (assuming neither <summary> nor <details> are absolute containing blocks)?  Does our behavior match Chrome there?

::: layout/base/nsCSSFrameConstructor.cpp:3532
(Diff revision 1)
> +    SIMPLE_TAG_CREATE(summary, NS_NewSummaryFrame)

How does this handle values of "overflow" other than "visible" on <summary>?  Please add tests.

::: layout/base/nsCSSFrameConstructor.cpp:5432
(Diff revision 1)
> +    return false;

Why is the right place to do this check here and not in AddFrameConstructionItemsInternal where other such checks are done?  In particular, why do we want to skip SetAsUndisplayedContent in this case?  I strongly suspect we do not in fact want that.

::: layout/base/nsCSSFrameConstructor.cpp:7675
(Diff revision 1)
> +    // and let it figures out where this summary element should be laid out.

"let it figure"

::: layout/base/nsCSSFrameConstructor.cpp:9170
(Diff revision 1)
> +    // When we remove a summary frame which is served as the summary content for

s/is served/serves/

::: layout/style/html.css:776
(Diff revision 1)
> +  display: list-item !important;

I think this is technically a spec violation, but so much of the "details binding" stuff is underdefined that maybe it's ok.

::: layout/style/html.css:797
(Diff revision 1)
> +  display: block !important;

You may want to inherit/override more things here.  Specifically at least inherit text-overflow and overflow-clip-box.  Possibly for ::-moz-details-content also need the "block-size: 100%" that ::-moz-fieldset-content has.  This part is not clear to me; in particular this depends on how "height: 100%" on kids should work when the <details> has padding and a summary.

Comment 145

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689430 [details]
MozReview Request: Bug 591737 - Provide a default summary element by DetailsFrame

https://reviewboard.mozilla.org/r/25607/#review23093

::: layout/generic/DetailsFrame.cpp:103
(Diff revision 1)
> +  // element lacks any direct <summry> child.

s/summry/summary/

r=me

Comment 146

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

Actually, wait.  r-, because this is not firing the "toggle" events the spec calls for when the "open" attribute is added/removed.

Comment 147

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689434 [details]
MozReview Request: Bug 591737 - Implement toggling open details by mouse click

https://reviewboard.mozilla.org/r/25613/#review23095

::: dom/html/HTMLDetailsElement.h:31
(Diff revision 1)
> +  void ToggleOpen() { SetOpen(!Open()); }

This will need to be updated once SetOpen() can throw.  Probably just suppress the exception.

::: dom/html/HTMLSummaryElement.cpp:73
(Diff revision 1)
> +  DetailsFrame* detailsFrame = do_QueryFrame(details->GetPrimaryFrame());

So... the spec is no help here, but what should happen when a display:none <details> has a mouse event (synthetic, clearly) fired at its first <summary>?  Should the "open" attribute of the <details> change?

What does Chrome do here?  Ideally we would match them and raise a spec issue to define this stuff...

Comment 148

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689435 [details]
MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

I'm going to punt this one to smaug.

Olli, the context is that the click default action posts a reframe for the parent of the (anonymous) node we're clicking on.  Then presumably something flushes out style/layout before the doubleclick fires (probably the click event processing for the second click) and the anonymous node is destroyed...

Comment 149

[Boris Zbarsky [:bzbarsky]]

Oh, one more question.  Have you done any testing with pagination/columnation?  What happens if the <details> or its content insertion kid split across a page/column boundary?  Seems like if that happens the "get my content frame" code that gets the last child would be wrong...  but I also didn't see any implementation of continuation frame creation in the frame constructor code, so not sure how this will actually behave in practice.  Please test.

Comment 150

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8689422 [details]
MozReview Request: Bug 591737 - Teach parser about <details> and <summary>

https://reviewboard.mozilla.org/r/25591/#review23119

Comment 151

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Re comment 149: I haven't done any test with pagination/columnation yet. Will investigate these as well as other properties like overflow, etc. described in comment 144.

Re comment 146: I filed bug 1225412 for firing 'toggle' event for details. Is it OK to get the layout right in this bug and deal with the event later?

Comment 156

[Boris Zbarsky [:bzbarsky]]

> Is it OK to get the layout right in this bug and deal with the event later?

Yes, as long as we make sure to not ship before the event bit is done.

Comment 157

[Lawrence Mandel [:lmandel] (use needinfo)]

Release Note Request (optional, but appreciated)
[Why is this notable]: The <details> is used as a disclosure widget from which the user
can obtain additional information or controls. <summary> is used as a summary or legend of the details. To expand the details, the user could click on the summary or by adding a bool attribute 'open' to the details.
[Suggested wording]: Support for details and summary HTML elements.
[Links (documentation, blog post, etc)]:
https://html.spec.whatwg.org/multipage/forms.html#the-details-element
https://groups.google.com/forum/#!topic/mozilla.dev.platform/b0UMDIasfq8

Comment 158

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8689435 [details]
MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

-      if (NS_SUCCEEDED(ret) && aEvent->clickCount == 2) {
+      if (NS_SUCCEEDED(ret) && aEvent->clickCount == 2 &&
+          mouseContent->GetComposedDoc()) {

Use IsInComposedDoc()

Comment 159

[Sebastian Zartner [:sebo]]

Regarding the release note request from comment 157, it should rather be requested for bug 1226455.

Sebastian

Comment 160

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689423 [details]
MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25593/diff/1-2/

Comment 161

[(no longer active)]

Comment on attachment 8689423 [details]
MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils

https://reviewboard.mozilla.org/r/25593/#review23385

Comment 162

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25601/#review23075

> Why are you handling this via AttributeChanged on the frame instead of just having your element implement GetAttributeChangeHint appropriately?  The latter would be far preferable, since it will coalesce much better with other style changes.

I simply do not know I should implement HTMLDetailsElement::GetAttributeChangeHint(). I'll change it in next patch set.

Comment 163

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25605/#review23087

If the <details> is taken out of line, it's behavior should like a normal block div. The behavior of my implementation should align with Chrome's.

> Have you tested what happens here with floats inside the <summary>?  I _think_ it should work because <summary> is forced to a block-like display so they get parented to it, not to the anonymous detailsContentFrame, but please make sure you have tests.
> 
> Also, I assume you have tests for display:inline styling on <details> and the like, right?  If you don't, please add them.
> 
> Have you tested what happens with:
> 
>   <details style="height: 200px">
>     <summary style="height: 100%">Stuff</summary>
>   </details>
> 
> as well as:
> 
>   <details style="height: 200px">
>     <div style="height: 100%">Stuff</summary>
>   </details>
> 
> ?  I see nothing in the reftests testing this sort of thing, and while the spec here is useless I would expect that the 100% height nodes in those testcases would end up with a 200px height.
> 
> Have you tested how this affects z-ordering of absolutely positioned elements, where one is a child of the <summary> and one is a child of the <details> that comes before <summary> in the DOM (assuming neither <summary> nor <details> are absolute containing blocks)?  Does our behavior match Chrome there?

I add all test cases mentioned in patchset 2, including floats inside the <summary>, absolute height children, and z-order test.

Q: Have you tested how this affects z-ordering of absolutely positioned elements, where one is a child of the <summary> and one is a child of the <details> that comes before <summary> in the DOM (assuming neither <summary> nor <details> are absolute containing blocks)?  Does our behavior match Chrome there?
A: The child of <summary> will be under the the child of <details> assuming above condition. Our behavior matches both Chrome and Safari.

> I think this is technically a spec violation, but so much of the "details binding" stuff is underdefined that maybe it's ok.

Instead of setting summary element as list-item, I construct nsBulletFrame in SummaryFrame for the disclosure triangle in patchset 2.

Comment 164

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25605/#review23087

> Why is the right place to do this check here and not in AddFrameConstructionItemsInternal where other such checks are done?  In particular, why do we want to skip SetAsUndisplayedContent in this case?  I strongly suspect we do not in fact want that.

I move the check to AddFrameConstructionItemsInternal() and add undisplyed items by using SetAsUndisplayedContent() in patchset 2.

> You may want to inherit/override more things here.  Specifically at least inherit text-overflow and overflow-clip-box.  Possibly for ::-moz-details-content also need the "block-size: 100%" that ::-moz-fieldset-content has.  This part is not clear to me; in particular this depends on how "height: 100%" on kids should work when the <details> has padding and a summary.

I simplified the frame structure by removing the two anonymous block children from details frame in patchset 2. This is not needed.

Comment 165

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25613/#review23095

> So... the spec is no help here, but what should happen when a display:none <details> has a mouse event (synthetic, clearly) fired at its first <summary>?  Should the "open" attribute of the <details> change?
> 
> What does Chrome do here?  Ideally we would match them and raise a spec issue to define this stuff...

Both Chrome and Safari do not touch the 'open' attribute if the details has display: none. We'd better follow their behavior here. I've added a test case mouse-click-display-none-details.html. Also filed a spec issue. https://github.com/whatwg/html/issues/517

Comment 209

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Due to mozreview server error for my patchset 2, I push my patchset again. The "patchset 2" mentioned in comment 163 and comment 164 should now be refer to "patchset 3".

Comment 210

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Olli, you had grant r+ for the patch 
"Bug 591737 - Avoid dispatch mouse double click to content not in doc" in comment 158, but the r+ does not recorded in mozreview. Would you please click "ship it" for me in mozreview again? Thanks.

Comment 211

[Olli Pettay [:smaug][[e-mail]]]

done. it is unfortunate that bugzilla<->mozreview integration doesn't quite work.

Comment 212

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8689435 [details]
MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

https://reviewboard.mozilla.org/r/25615/#review28359

Comment 213

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

bz, this bug needs your review to move forward. Thanks.

Comment 214

[Boris Zbarsky [:bzbarsky]]

Huh.  I thought I had commented on this one back when review was requested...

It's been a long time since I last looked at this stuff, and I need to page it back in.  Which means I need to find a multi-hour-long chunk of time to look at this effectively.

This is pretty high on my priority list, but scheduling around various meetings has been complicated.  I'm certainly not going to get to this today, but hoping to on Monday or Tuesday.  I'm sorry for the lag...

Comment 215

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

bz, thank you for consider this feature high priority. I take me a long time to address all the comments and add the tests, and I hope I'm on the right track :)

Comment 216

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

https://reviewboard.mozilla.org/r/25595/#review29845

::: dom/webidl/HTMLDetailsElement.webidl:14
(Diff revisions 1 - 3)
>  interface HTMLDetailsElement : HTMLElement {

This needs to not be exposed until the toggle thing is fixed.  And you need to not have the parser creating this new class until then either.

Comment 217

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689427 [details]
MozReview Request: Bug 591737 - Add DetailsFrame

https://reviewboard.mozilla.org/r/25601/#review29849

::: layout/generic/DetailsFrame.cpp:46
(Diff revision 3)
> +DetailsFrame::SetInitialChildList(ChildListID aListID, nsFrameList& aChildList)

Why does this function exist?

Comment 218

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689428 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

https://reviewboard.mozilla.org/r/25603/#review29851

::: layout/generic/SummaryFrame.cpp:35
(Diff revisions 1 - 3)
> +SummaryFrame::SetInitialChildList(ChildListID aListID, nsFrameList& aChildList)

Again, why does this function exist?

Comment 219

[Boris Zbarsky [:bzbarsky]]

https://reviewboard.mozilla.org/r/25605/#review23087

What in your code makes it behave like a normal block?  I don't see anything to do that.

Also, I might have meant <summary>, not <details>, above.  Try this testcase in Chrome:

<!DOCTYPE html>
<body>
  Some text here.
  <details>
    <summary style="position: absolute; top:0; right: 0; width: 100px; border:
                    1px solid green">aaa</summary>
    bbb
  </details>
</body>

and see whether that's the behavior your patch has (it's not) and then we can think about what behavior would actually make sense here.

> I add all test cases mentioned in patchset 2, including floats inside the <summary>, absolute height children, and z-order test.
> 
> Q: Have you tested how this affects z-ordering of absolutely positioned elements, where one is a child of the <summary> and one is a child of the <details> that comes before <summary> in the DOM (assuming neither <summary> nor <details> are absolute containing blocks)?  Does our behavior match Chrome there?
> A: The child of <summary> will be under the the child of <details> assuming above condition. Our behavior matches both Chrome and Safari.

OK.  That probably needs a spec issue raised, then.  Please do that.

Comment 220

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689429 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

https://reviewboard.mozilla.org/r/25605/#review29859

::: dom/html/HTMLDetailsElement.cpp:24
(Diff revision 3)
> +  // XXX: Might want to cache the first summary element.

Might be a decent idea.  Please file a followup.

::: layout/base/nsCSSFrameConstructor.cpp:11508
(Diff revision 3)
> +                blockFrame->GetType() == nsGkAtoms::detailsFrame),

Can summaries get columns?  Followup probably OK for that.

::: layout/generic/DetailsFrame.cpp:48
(Diff revision 3)
> +  if (aListID == kPrincipalList) {

Ah, this is why you needed this function...

::: layout/generic/DetailsFrame.cpp:53
(Diff revision 3)
> +      // If details is open, the first summary need to be render as if it is the

"needs to be rendered"

r=me.  This is much better, thank you.

Comment 221

[Boris Zbarsky [:bzbarsky]]

> What in your code makes it behave like a normal block?  I don't see anything to do that.

I guess the upshot is that absolutely positioning or floating these things will just make them act like blocks from the outside, but still use their special frame classes.  OK.

Comment 222

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689434 [details]
MozReview Request: Bug 591737 - Implement toggling open details by mouse click

https://reviewboard.mozilla.org/r/25613/#review29863

::: dom/html/HTMLDetailsElement.h:52
(Diff revision 3)
> +  }

You do need to rv.SuppressException() here.

::: dom/html/HTMLSummaryElement.cpp:55
(Diff revision 3)
> +  if (details->GetPrimaryFrame()) {

So I realized that my "clearly synthetic" comment is wrong, as I commented in the W3C issue.  Also of interest are cases in which a capturing event listener higher up the DOM sets the <details> or <summary> to display:none. Or moves the <summary> to a different <details> element, for that matter.  Please check what Chrome/Safari do in all three of those situations?

Comment 223

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689437 [details]
MozReview Request: Bug 591737 - Add reftest for mouse click on summary

https://reviewboard.mozilla.org/r/25617/#review29865

::: layout/reftests/details-summary/fixed-summary.html:1
(Diff revision 3)
> +<!DOCTYPE html>

layout/reftests/details-summary/fixed-summary.html seems to not be used for anything?

::: layout/reftests/details-summary/float-details.html:1
(Diff revision 3)
> +<!DOCTYPE html>

layout/reftests/details-summary/float-details.html seems to not be used for anything?

::: layout/reftests/details-summary/mouse-click-fixed-summary.html:1
(Diff revision 3)
> +<!DOCTYPE html>

layout/reftests/details-summary/mouse-click-fixed-summary.html seems to not be used for anything?

::: layout/reftests/details-summary/mouse-click-fixed-summary.html:16
(Diff revision 3)
> +    summary.dispatchEvent(new MouseEvent("click"));

Spec editor says this should not toggle, of course (but I think spec editor is wrong).

In general, there's a bunch of files being added here that are not being added to the reftest.list, so this changeset is not making any sense to me.

Comment 224

[Xidorn Quan [:xidorn] UTC+10]

FWIW, I believe tests for <details> and <summary> should go to web-platform tests.

Comment 225

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8709973 [details]
MozReview Request: Bug 591737 - Add pref for details and summary elements

https://reviewboard.mozilla.org/r/31615/#review29867

Ah, here's the disabling for now.  r=me and ignore my earlier comments about this being needed, since it's clearly done.  :)

Comment 226

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689432 [details]
MozReview Request: Bug 591737 - Add crashtest for details and summary

https://reviewboard.mozilla.org/r/25611/#review29869

Probably a good idea to throw some columns and scrollability in here too, right?

r=me with those added.

Comment 227

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

https://reviewboard.mozilla.org/r/25595/#review29871

r=me since the pref gets added later.

Comment 228

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689427 [details]
MozReview Request: Bug 591737 - Add DetailsFrame

https://reviewboard.mozilla.org/r/25601/#review29873

r=me since this function is in fact used later.  A comment to that effect in this changeset would have been nice, or just not adding it until it was used... ;)

Comment 229

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689428 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

https://reviewboard.mozilla.org/r/25603/#review29875

Comment 230

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689431 [details]
MozReview Request: Bug 591737 - Add reftests for details and summary

https://reviewboard.mozilla.org/r/25609/#review29877

::: layout/reftests/details-summary/details-absolute-children.html:39
(Diff revision 3)
> +      <div id="before_summary">div before summary</div> <!-- This div is at front. -->

Please do make sure that a spec issue is filed for this.

::: layout/reftests/details-summary/details-in-ol.html:13
(Diff revision 3)
> +    <!-- Test <ol> numbering does not affect by <details>. -->

s/does not affect/is not affected/

::: layout/reftests/details-summary/details-page-break-before-2.html:8
(Diff revision 3)
> +      <summary>Summary<div style="page-break-after: always;"></div></summary>

This file is identical to details-page-break-after-2.html as far as I can tell.

::: layout/reftests/details-summary/details-three-columns.html:21
(Diff revision 3)
> +  div#columns {

Don't need this style here, right?

::: layout/reftests/details-summary/details-three-columns.html:40
(Diff revision 3)
> +    </div>

Should be </details>

::: layout/reftests/details-summary/details-two-pages.html:1
(Diff revision 3)
> +<!DOCTYPE html>

details-two-pages.html seems to be identical to details-page-break-after-1.html, no?

It would be better to use <div>s for the reference (and suppress the disclosure triangle in the relevant tests in the usual way).

It would also be good to have copies of these page-break tests with overflow:auto on the details and summary, but you should probably do that in a followup, because I suspect they don't do the magic we do for scrollable blocks in printing and adding it might be a bit complicated.

::: layout/reftests/details-summary/float-in-summary.html:30
(Diff revision 3)
> +      <summary><h4>This is float: right in summary</h4>Lorem ipsum dolor sit amet,

The reference has a line break before the <h4>.  The test should too.

::: layout/reftests/details-summary/overflow-auto-details.html:1
(Diff revision 3)
> +<!DOCTYPE html>

overflow-auto-details.html is not in reftest.list that I can see.

::: layout/reftests/details-summary/overflow-hidden-details.html:1
(Diff revision 3)
> +<!DOCTYPE html>

overflow-hidden-details.html isn't in reftest.list

Not marking r+ because of the files missing from reftest.list and the few other substantive issues above that I want to see the changes for.

Comment 231

[Boris Zbarsky [:bzbarsky]]

And my apologies again for the lag.  Hopefully the next round-trip here will be much quicker on both our parts!

Comment 232

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25605/#review29859

> Might be a decent idea.  Please file a followup.

Filed bug 1245032.

> Can summaries get columns?  Followup probably OK for that.

Currently, summaries do not support columns. Filed bug 1245036

> "needs to be rendered"

Will fix in next patch.

Comment 233

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25601/#review29873

Sorry for the confusion. I'll move DetailsFrame::SetInitialChildList() and SummaryFrame::SetInitialChildList() to "Bug 591737 - Construct details and summary in nsCSSFrameConstructor" in next patchset.

Comment 234

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Xidorn Quan [:xidorn] (UTC+10) (less responsive until Feb 22) from comment #224)
> FWIW, I believe tests for <details> and <summary> should go to web-platform
> tests.

xidorn, thanks for the suggestion. Filed bug 1245047.

Comment 235

[Boris Zbarsky [:bzbarsky]]

After sleeping on this, it might also be worth testing how 

  <details style="display:flex">
    <summary></summary>
    <div></div>
  </details>

and

  <details style="display: table">
    <summary style="display: table-cell"></summary>
    <div style="display: table-cell"></div>
  </details>

work in your implementation and in other browsers.  Please file followup bugs and spec issues as needed based on that testing.

Comment 236

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25605/#review23087

> I guess the upshot is that absolutely positioning or floating these things will just make them act like blocks from the outside, but still use their special frame classes.  OK.

Your guess is correct. The absolute positioning or floating summary acts like block from outside, and can still be used to toggle details by mouse. Both Chrome and my implementation behave the same way. The frame tree dump of the example from comment 219 is here. https://pastebin.mozilla.org/8858331

> OK.  That probably needs a spec issue raised, then.  Please do that.

Filed a spec issue about the z-order in https://github.com/whatwg/html/issues/603

Comment 237

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25617/#review29865

> Spec editor says this should not toggle, of course (but I think spec editor is wrong).

I guess it's OK for now that the MouseEvent still toggle.

Somehow those reftests for 'fixed' and 'float' are missing from reftest.list during my patch rewriting ... I'll add them back. Thanks for pointing this out.

Comment 238

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25609/#review29877

> Please do make sure that a spec issue is filed for this.

Sure. Filed https://github.com/whatwg/html/issues/603

> This file is identical to details-page-break-after-2.html as far as I can tell.

Nice catch. It should be page-break-after: always.

> Don't need this style here, right?

You're right. Removed div#columns.

> details-two-pages.html seems to be identical to details-page-break-after-1.html, no?
> 
> It would be better to use <div>s for the reference (and suppress the disclosure triangle in the relevant tests in the usual way).
> 
> It would also be good to have copies of these page-break tests with overflow:auto on the details and summary, but you should probably do that in a followup, because I suspect they don't do the magic we do for scrollable blocks in printing and adding it might be a bit complicated.

You're right. Using <div>s for the refernce is better. Filed bug 1245395 for adding page-break tests with overflow:auto.

> The reference has a line break before the <h4>.  The test should too.

Fixed in next patch.

> overflow-auto-details.html is not in reftest.list that I can see.

It's used in "Bug 591737 - Add reftest for mouse click on summary". I'll move it.

> overflow-hidden-details.html isn't in reftest.list

It's used in "Bug 591737 - Add reftest for mouse click on summary". I'll move it.

Comment 239

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://reviewboard.mozilla.org/r/25613/#review29863

> You do need to rv.SuppressException() here.

Will do in next patchset.

> So I realized that my "clearly synthetic" comment is wrong, as I commented in the W3C issue.  Also of interest are cases in which a capturing event listener higher up the DOM sets the <details> or <summary> to display:none. Or moves the <summary> to a different <details> element, for that matter.  Please check what Chrome/Safari do in all three of those situations?

Curretly, display:none summmary crashes, so filed bug 1245424 for a follow-up. I guess we still need to be compatible with Chrome/Safari in these three cases.

Comment 240

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Boris Zbarsky [:bz] from comment #235)
> After sleeping on this, it might also be worth testing how 
> 
>   <details style="display:flex">
>     <summary></summary>
>     <div></div>
>   </details>
> 
> and
> 
>   <details style="display: table">
>     <summary style="display: table-cell"></summary>
>     <div style="display: table-cell"></div>
>   </details>
> 
> work in your implementation and in other browsers.  Please file followup
> bugs and spec issues as needed based on that testing.

Filed bug 1245430 and bug 1245431 for support display:flex and display:table, respectively.

Comment 241

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689422 [details]
MozReview Request: Bug 591737 - Teach parser about <details> and <summary>

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25591/diff/3-4/

Comment 242

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689423 [details]
MozReview Request: Bug 591737 - Add details and summary to nsHTMLEditUtils

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25593/diff/3-4/

Comment 243

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689424 [details]
MozReview Request: Bug 591737 - Add HTMLDetailsElement and webidl interface

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25595/diff/3-4/

Comment 244

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689425 [details]
MozReview Request: Bug 591737 - Add HTMLSummaryElement

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25597/diff/3-4/

Comment 245

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689427 [details]
MozReview Request: Bug 591737 - Add DetailsFrame

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25601/diff/3-4/

Comment 246

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689426 [details]
MozReview Request: Bug 591737 - Add SummaryFrame

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25599/diff/3-4/

Comment 247

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689428 [details]
MozReview Request: Bug 591737 - Construct details and summary in nsCSSFrameConstructor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25603/diff/3-4/

Comment 248

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689430 [details]
MozReview Request: Bug 591737 - Provide a default summary element by DetailsFrame

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25607/diff/3-4/

Comment 249

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689431 [details]
MozReview Request: Bug 591737 - Add reftests for details and summary

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25609/diff/3-4/

Comment 250

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689432 [details]
MozReview Request: Bug 591737 - Add crashtest for details and summary

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25611/diff/3-4/

Comment 251

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689434 [details]
MozReview Request: Bug 591737 - Implement toggling open details by mouse click

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25613/diff/3-4/

Comment 252

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689435 [details]
MozReview Request: Bug 591737 - Avoid dispatch mouse double click to content not in doc

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25615/diff/3-4/

Comment 253

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689437 [details]
MozReview Request: Bug 591737 - Add reftest for mouse click on summary

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25617/diff/3-4/

Comment 254

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689438 [details]
MozReview Request: Bug 591737 - Fix test_HTMLSpec.html

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25619/diff/3-4/

Comment 255

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8689439 [details]
MozReview Request: Bug 591737 - Mark tests in web-platform-test pass

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25621/diff/3-4/

Comment 256

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8709973 [details]
MozReview Request: Bug 591737 - Add pref for details and summary elements

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31615/diff/2-3/

Comment 257

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689434 [details]
MozReview Request: Bug 591737 - Implement toggling open details by mouse click

https://reviewboard.mozilla.org/r/25613/#review30129

Comment 258

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689431 [details]
MozReview Request: Bug 591737 - Add reftests for details and summary

https://reviewboard.mozilla.org/r/25609/#review30131

Comment 259

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8689437 [details]
MozReview Request: Bug 591737 - Add reftest for mouse click on summary

https://reviewboard.mozilla.org/r/25617/#review30135

Comment 261

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c1a2e7c6191

Comment 262

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c3992d92f27
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e9190bcd7e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/e95439f4c6ce
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cee7af18f88
https://hg.mozilla.org/integration/mozilla-inbound/rev/95c721eb4e94
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4f84794091c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e62deaa9a733
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5e3c2b54b0e
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbe4d34c5019
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3f8a3303dbe
https://hg.mozilla.org/integration/mozilla-inbound/rev/804f641540a4
https://hg.mozilla.org/integration/mozilla-inbound/rev/98d74607f547
https://hg.mozilla.org/integration/mozilla-inbound/rev/afeae0daec33
https://hg.mozilla.org/integration/mozilla-inbound/rev/fed3c367cdd5
https://hg.mozilla.org/integration/mozilla-inbound/rev/96ba4ef5ffc2
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9585cb5a3cf

Comment 263

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8c3992d92f27
https://hg.mozilla.org/mozilla-central/rev/4e9190bcd7e4
https://hg.mozilla.org/mozilla-central/rev/e95439f4c6ce
https://hg.mozilla.org/mozilla-central/rev/5cee7af18f88
https://hg.mozilla.org/mozilla-central/rev/95c721eb4e94
https://hg.mozilla.org/mozilla-central/rev/a4f84794091c
https://hg.mozilla.org/mozilla-central/rev/e62deaa9a733
https://hg.mozilla.org/mozilla-central/rev/d5e3c2b54b0e
https://hg.mozilla.org/mozilla-central/rev/dbe4d34c5019
https://hg.mozilla.org/mozilla-central/rev/f3f8a3303dbe
https://hg.mozilla.org/mozilla-central/rev/804f641540a4
https://hg.mozilla.org/mozilla-central/rev/98d74607f547
https://hg.mozilla.org/mozilla-central/rev/afeae0daec33
https://hg.mozilla.org/mozilla-central/rev/fed3c367cdd5
https://hg.mozilla.org/mozilla-central/rev/96ba4ef5ffc2
https://hg.mozilla.org/mozilla-central/rev/c9585cb5a3cf

Comment 264

[Sebastian Zartner [:sebo]]

Added description:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details

Not added to notes for developers yet. Will be done in bug 1226455.

Sebastian

Comment 265

[Boris Zbarsky [:bzbarsky]]

Note that the support is not actually enabled by default yet (it's behind a pref) and it's not clear whether that will happen for 47.  How can we make MDN reflect that?

Comment 266

[Sebastian Zartner [:sebo]]

(In reply to Boris Zbarsky [:bz] from comment #265)
> Note that the support is not actually enabled by default yet (it's behind a
> pref) and it's not clear whether that will happen for 47.  How can we make
> MDN reflect that?

I added a hint on the two pages that they are implemented behind a preference and I added now that those two elements are only available on Nightly releases.

Sebastian

Comment 267

[Boris Zbarsky [:bzbarsky]]

Thanks!

Comment 268

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added to Aurora47 release notes.

Comment 269

[Matthew N. [:MattN]]

Ritu, in case you didn't notice the implementation is behind the dom.details_element.enabled pref. Bug 1226455 is for shipping without the pref.

Comment 270

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #269)
> Ritu, in case you didn't notice the implementation is behind the
> dom.details_element.enabled pref. Bug 1226455 is for shipping without the
> pref.

Thanks Matt! I have linked to the MDN articles as the pref is mentioned there.

Comment 271

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This shouldn't be in the release notes until it's actually enabled.  Could you remove it?

Comment 272

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Ok. I can do that. We can add it back when they are pref enabled I suppose.

Comment 273

[wig_2006]

Thanks you to result , I have victory to support HTML5's <details> and <summary>.

Comment 274

[Mike Gifford]

So is this in FF 46.0 now? Not sure what to pull out of the last few messages here.

Comment 275

[Boris Zbarsky [:bzbarsky]]

Mike, the code is checked into what will become Firefox 47, but it's not enabled there yet, because it has some issues still.

In Firefox 48, the code is enabled on nightly and developer edition only, not in final release.

The current plan is to enable in release in Firefox 49.  See bug 1226455.

Comment 276

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Marcia, could you please add this to Fx49 relnotes instead and update the relnote tracking flag to 49+? Thanks!

Comment 277

[Marcia Knous [:marcia]]

(In reply to Ritu Kothari (:ritu) from comment #276)
> Hi Marcia, could you please add this to Fx49 relnotes instead and update the
> relnote tracking flag to 49+? Thanks!

This was already listed in the Developer Notes on MDN: https://developer.mozilla.org/en-US/Firefox/Releases/49#Changes_for_Web_developers. Adding 49+ flag for relnote.