267612 - Implement <label>

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Brian Ryner (not reading)]

As the summary says.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

This implements <label> for input elements.  To avoid more code duplication, I
got rid of all existing code that gets or sets instance data and reimplemented
it in nsXFormsUtils.  I was very careful to follow the spec to the letter on
this...

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 164525 [details] [diff] [review]
patch

marking obsolete, I'm going to comment this code a little more.

Comment 3

[Brian Ryner (not reading)]

There's an alternate approach to this that I'd considered, where instead of the
label telling the parent form control to set the label text, the parent asks the
label for it when the control is refreshed.  With that appraoch I'd say it would
be difficult to respond to any dynamic changes to the <label> via the DOM, but
it seems like it would be a bit more efficient otherwise.  This gets back to the
whole issue of whether we attempt to deal with dynamic updates as they happen,
or rely on the author to dispatch the correct XForms events when manipulating
the DOM.

Allan, any thoughts on this?

Comment 4

[Darin Fisher]

r=me for nsXFormsUtils::Get/SetNodeValue

in GetNodeValue, you might want to move the Truncate(0) calls outside of the
switch statement and down to the end of the function.  then make any of the
cases that actually set the out-param to do a return instead of a break.  also,
you can just call Truncate() without any arguments.

Comment 5

[Robert Marcano]

Trying to solve this in a generic way I implemented this method on nsXFormsControl

  /**
   * Install all the event listeners to the label element and returns the generated
   * content of it (label support output childrens so the generated element)
   */
  NS_HIDDEN_(nsresult) InitLabelElement(nsCOMPtr<nsIDOMDocumentFragment>&
aFragment);

this method looks for the label element and if found return the HTML visual
representation of it as a DocumentFragment. This method must be called when the
element visual is created. For dynamic changes the event handlers call a virtual
function named UpdateLabel that must be implemented on nsXFormsControl children

One thing that is important to remember is that xf:label can have xf:output children

Comment 6

[Robert Marcano]

reading the obsoleted patch I noted that setLabel has this signature

nsXFormsInputElement::SetLabel(const nsAString &aLabel)

so, it is assuming that the entire label is a String, with that in mind the
following example will need to be converted to a single string:

<xf:label>Name for <xf:output ref="apath"/></xf:label>

That is not a problem, unless the author wants to style the output elements with
for example a different color. a DocumentFragment with html span elements can
help with this, asuming that all the controls will show the labels as HTML

Comment 7

[Leigh L. Klotz, Jr.]

I support the goal of having label work with markup instead of strings.  Many
XForms authors use HTML (including img elements) inside <label>, and make
extensive use of CSS.  Authors need as much flexibility as possible with labels,
given that XForms enforces the label as a child of the control element.  If
label can't handle markup or fails to take advantage of CSS, too hard to use,
they will put in <label/> and use markup elsewhere for the real label, to the
detriment of accessibility.  Also note that the label contents need not be
limited to HTML markup -- if the host processor understands SVG or other markup
languages, they ought to work too.

Comment 8

[Darin Fisher]

> I support the goal of having label work with markup instead of strings.  Many
> XForms authors use HTML (including img elements) inside <label>, and make
> extensive use of CSS.

Hmm, in section 8.1 of the XForms spec, <label>'s minimal content model is
defined as: (PCDATA|(UI Inline))*, where UI Inline's minimal content model is
defined as (output)*.  I presume that does not restrict <label> from having
XHTML <img> elements as children (since the XForms spec only talks of a minimal
content model), but I don't see a specification that covers how that should be
done.  Is there a specification?

Comment 9

[aaronr]

Here is a dialog between me and T.V. Raman getting his input on the results of
an ongoing dialog with me and Aaron Leventhal about how to handle the src
attribute on Label (src pointing to an image):

Aaron> Hey T.V.
    Aaron> 
    Aaron> This is what Aaron Leventhal and I are thinking as far
    Aaron> as how Mozilla Xforms would handle encountering a
    Aaron> label element with the src attribute pointing to an
    Aaron> image.  We are taking this as a scenario from the
    Aaron> overall issue to see how the browser and a Screen
    Aaron> Reader would handle it.
    Aaron> 
    Aaron> Here are the two ways that we think it can be managed:
    Aaron> When the label's src attribute points to an image
    Aaron> file, we render the image. The accessibility API
    Aaron> implementation then associate the inline text for the
    Aaron> image's accessible name.  We don't render an image
    Aaron> pointed to by a label's src attribute at all, to avoid
    Aaron> situations where authors don't feel like inserting
    Aaron> alternative inline text.
    Aaron> 
    Aaron> Again, I know that this is a small piece of the
    Aaron> overall puzzle, but it shows the kinds of things that
    Aaron> a browser can run up against.  Maybe a suggestion
    Aaron> would be that if there is a src attribute and there is
    Aaron> also inline text that it not necessarily be ignored,
    Aaron> but rather treated as alternate text?  By making
    Aaron> special mention of it, maybe this way xforms tooling
    Aaron> would allow both to exist on a label element rather
    Aaron> than showing a warning or an error to the author if
    Aaron> the author tries to supply both.  Just a thought.

Agreed. In fact src="image" should be depricated as far as
possible; it's basically just an edge case to discuss hwo things
work. 
Note that from memory the XForms spec says that the content of
element label is fallback if src cannot be displayed; and that is
what I'd expect an access agent to do. The filename is a final
fall-down option -- but the use of src being an image would
really be for the CSSZenGarden type of use -- incidentally that
is the intent behind p src="foo">...</p> in XHTML2 as well.

Comment 10

[Robert Marcano]

(In reply to comment #8)
> Hmm, in section 8.1 of the XForms spec, <label>'s minimal content model is
> defined as: (PCDATA|(UI Inline))*, where UI Inline's minimal content model is
> defined as (output)*.  I presume that does not restrict <label> from having
> XHTML <img> elements as children (since the XForms spec only talks of a minimal
> content model), but I don't see a specification that covers how that should be
> done.  Is there a specification?

I am not sure about the intention of the spec authors when they define label as
(PCDATA|(UI Inline))*

If you check the normative XForms schema
(http://www.w3.org/MarkUp/Forms/2002/XForms-Schema.xsd) you will get this
definition of label

  <xsd:element name="label">
    <xsd:complexType mixed="true">
      <xsd:group ref="xforms:UI.Inline"/>
      <xsd:attributeGroup ref="xforms:Common.Attributes"/>
      <xsd:attributeGroup ref="xforms:Single.Node.Binding.Attributes"/>
      <xsd:attributeGroup ref="xforms:Linking.Attributes"/>
    </xsd:complexType>
  </xsd:element>

so the only allowed children are CDATA (mixed="true") and UI.Inline elements.
UI.Inline is defined this way

  <xsd:group name="UI.Inline">
    <xsd:sequence>
      <xsd:choice minOccurs="0">
        <xsd:element ref="xforms:output"/>
        <!-- containing document language to add additional allowed content here -->
      </xsd:choice>
    </xsd:sequence>
  </xsd:group>

So it is expected that the containing document language (in our case XHTML) can
define more inline elements to be allowed. Maybe the XHTML2 and XForms "merge"
will define that

Comment 11

[Brian Ryner (not reading)]

Darin and I talked this over and we think an easier way to do this is simply to
make labels supply a <span> as their anonymous content.  The span could have an
inner span that is used as the XTF insertion point if we decide to use inline
content, otherwise, it could be visibility: hidden.  The advantage of this
approach is that there is no different in how output works depending on whether
it is an outermsot element or contained inside a label; it always just supplies
a span.

If one of our controls is unable to accomodate a span as a label (I'm not aware
of any such cases at the moment), it could extract the text from the span.  I
agree with Robert and Leight that we should treat label content as pure inline
content rather than plain text wherever practical.

Unfortunately, my attempt to do this exposes bug 269023.

Comment 12

[Allan Beaufour]

I opt for the last strategy, using a span.

Comment 13

[Brian Ryner (not reading)]

Attachment: label implementation

Comment 14

[Robert Marcano]

Attachment: input with label test

Test case where the second patch does not works for the input element

Comment 15

[Robert Marcano]

the same attachment #166145 [details] shows a problem with output elements inside a label
ones, the content of the "output" is not shown

Comment 16

[Darin Fisher]

Robert: I think that is expected.  See bug 269023.  Once that is fixed, your
testcase should work.

Comment 17

[Darin Fisher]

Comment on attachment 166132 [details] [diff] [review]
label implementation

>Index: nsXFormsLabelElement.cpp

>+  // owning reference to this object, so as long as wel null out mElement in

s/wel/we/


>+  node->GetOwnerDocument(getter_AddRefs(domDoc));
>+
>+  domDoc->CreateElementNS(NS_LITERAL_STRING(NS_NAMESPACE_XHTML),

IIRC, you said that it is possible for GetOwnerDocument to return NULL in
some cases, so perhaps we should null check it before dereferencing?


r=darin

Comment 18

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

I'm not so sure the test case will work.
nsXFormsLabelElement does not create an insertion point, so
it will not render its children. Or am I missing something here?
(node3->GetTextContent(labelValue); is not enough. )

Comment 19

[Brian Ryner (not reading)]

(In reply to comment #18)
> I'm not so sure the test case will work.
> nsXFormsLabelElement does not create an insertion point, so
> it will not render its children. Or am I missing something here?
> (node3->GetTextContent(labelValue); is not enough. )
> 

You're right.  I will post a new patch that addresses this.  Basically, what I'd
like to do is have the anonymous content look like this:

<span>
  textnode
  <span/>
</span>

The inner span is the insertion point, and is dynamically shown or hidden based
on whether we're currently showing the inline content.  If we're bound to
instance data, we hide the span and put the value from the instance data into
the text node.

While testing this, I discovered bug 270899.  I'll attach a testcase that shows
this.  I think this type of DHTML manipulation is a bit of an edge case though
and it shouldn't block landing label support.

Comment 20

[Brian Ryner (not reading)]

Attachment: testcase for inline and instance label content

Comment 21

[Brian Ryner (not reading)]

Attachment: revised patch

This patch works with the above testcase, except for the bug I noted.

Comment 22

[Darin Fisher]

Comment on attachment 166528 [details] [diff] [review]
revised patch

r=darin

Comment 23

[Brian Ryner (not reading)]

checked in