687393 - HTML select options gets relation from containing label

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Example:

<label>Hello
<select>
  <option>option</option>
</select>

Example in web: http://harkin.senate.gov/contact_opinion.cfm

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

based on bug 673958

Comment 2

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 3

[Trevor Saunders (:tbsaunde)]

Comment on attachment 560864 [details] [diff] [review]
patch2


>+  // document.
>+  nsAccessible* walkUp = mAcc->Parent();
>+  while (walkUp->IsElement()) {
>+    nsIContent* walkUpElm = walkUp->GetContent();

If we really care about pef we should move the nsIContent* local var out of the loop so we can call  the GetContent() vfunc once per loop instead of twice.  (we call it twice here once explicitly and once in IsElement()).

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> Comment on attachment 560864 [details] [diff] [review] [diff] [details] [review]
> patch2
> 
> 
> >+  // document.
> >+  nsAccessible* walkUp = mAcc->Parent();
> >+  while (walkUp->IsElement()) {
> >+    nsIContent* walkUpElm = walkUp->GetContent();
> 
> If we really care about pef we should move the nsIContent* local var out of
> the loop so we can call  the GetContent() vfunc once per loop instead of
> twice.  (we call it twice here once explicitly and once in IsElement()).

I'll replace walkUp->IsElement() on !walkUp->IsDoc() that should be more pref and more clear what is it for.

Comment 5

[alexander :surkov (:asurkov)]

inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/da2b4e883854

Comment 6

[Michael Wu [:mwu]]

https://hg.mozilla.org/mozilla-central/rev/da2b4e883854

Comment 7

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

Comment on attachment 560864 [details] [diff] [review]
patch2

>--- a/accessible/src/base/AccIterator.cpp
>+++ b/accessible/src/base/AccIterator.cpp
> HTMLLabelIterator::Next()
> {
>   // Get either <label for="[id]"> element which explicitly points to given
>   // element, or <label> ancestor which implicitly point to it.
>   nsAccessible* label = nsnull;
>   while ((label = mRelIter.Next())) {
>     if (label->GetContent()->Tag() == nsGkAtoms::label)
>+    if (walkUpElm->Tag() == nsGkAtoms::label) {
>+    if (walkUpElm->Tag() == nsGkAtoms::form)

Is it safe to assume that these are in the HTML namespace? If so, I'd appreciate assertions.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to Ms2ger from comment #7)

> Is it safe to assume that these are in the HTML namespace? If so, I'd
> appreciate assertions.

It's sort of assumption it never happens in real life. It makes sense to keep HTML namespace for correctness.