1483787 - Implement fieldset/legend in terms of '-webkit-appearance'

Status: RESOLVED WONTFIX

(Core :: Layout, enhancement, P3)

Description

[Simon Pieters [:zcorpan]]

See proposal in https://github.com/whatwg/html/issues/3912

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This would need fixing bug 1435658 to make dynamic changes work properly.

Comment 2

[Mats Palmgren (inactive)]

Attachment: wip

I took a stab at implementing the legend part of the draft spec
to see if there are any implementation issues with it.

It seems fairly straightforward to implement (although we also
need to amend bug 1435658 for these values eventually).

This patch allows an arbitrary (in-flow) box to become
the rendered legend by setting -webkit-appearance:fieldset-legend.
It's a bit unfortunate that the spec chose that property though,
since setting it on <input> etc (that already have values in 
the UA sheet) loses its theme styling in that case, which seems
unfortunate.  So I wonder if a new property for this wouldn't be
better after all,  e.g. "legend: none | auto" so we don't
interfere with an element's normal appearance.

Thoughts?

Also, is there a way to put individual -webkit-appearance
values behind a pref?

(FTR, I suspect we can probably get rid of the nsLegendFrame class
as a follow-up after this, since it really doesn't do much.)

Comment 3

[Mats Palmgren (inactive)]

Also, this patch adds NS_BLOCK_FORMATTING_CONTEXT_STATE_BITS
*after* the <legend> frame subtree have been created rather
than at the time the nsLegendFrame is created (before its
kids).  So I'm making the assumption that that bit doesn't
influence frame construction of its descendants.
Is that a valid assumption?

Comment 4

[Mats Palmgren (inactive)]

Attachment: Basic -webkit-appearance: fieldset-legend test

Comment 5

[Mats Palmgren (inactive)]

Attachment: Screenshot of above testcase

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Mats Palmgren (:mats) from comment #2)
> Also, is there a way to put individual -webkit-appearance
> values behind a pref?

This can be done with #[parse(condition = "foo")], see the display values for an example.

(In reply to Mats Palmgren (:mats) from comment #3)
> Also, this patch adds NS_BLOCK_FORMATTING_CONTEXT_STATE_BITS
> *after* the <legend> frame subtree have been created rather
> than at the time the nsLegendFrame is created (before its
> kids).  So I'm making the assumption that that bit doesn't
> influence frame construction of its descendants.
> Is that a valid assumption?

I haven't seen anything that would break, but I haven't checked that carefully...

Comment 7

[Simon Pieters [:zcorpan]]

Does this need an intent to implement?

Comment 8

[Mats Palmgren (inactive)]

Attachment: wip2

Comment 9

[Mats Palmgren (inactive)]

(In reply to Simon Pieters from comment #7)
> Does this need an intent to implement?

I'd like the draft spec to stabilize a bit more before that.
In particular, I don't think '-webkit-appearance' is the right
property to use to trigger the rendered legend layout.
I've filed a spec issue:
https://github.com/whatwg/html/issues/3968

Comment 10

[Mats Palmgren (inactive)]

Note to self: dom/html/test/test_bug557087-3.html asserts:
MOZ_CRASH(nsFieldSetFrame::InsertFrames not supported) at layout/forms/nsFieldSetFrame.cpp:692

Comment 11

[Mats Palmgren (inactive)]

Attachment: wip3

Implemented legend:none|auto.
Put all the new stuff behind a pref: layout.css.new-fieldset-legend.enabled

Comment 12

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=21707c03032030a80f08906358a671cad3a43fda

Comment 13

[Mats Palmgren (inactive)]

Using appearance for this is probably not the right solution.