The default bug view has changed. See this FAQ.

HTML select options gets relation from containing label

RESOLVED FIXED in mozilla10

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla10
access
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

17.48 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Example:

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

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

Comment 1

6 years ago
Created attachment 560861 [details] [diff] [review]
patch

based on bug 673958
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #560861 - Flags: review?(trev.saunders)
(Assignee)

Comment 2

6 years ago
Created attachment 560864 [details] [diff] [review]
patch2
Attachment #560861 - Attachment is obsolete: true
Attachment #560861 - Flags: review?(trev.saunders)
Attachment #560864 - Flags: review?(trev.saunders)
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()).
Attachment #560864 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 4

6 years ago
(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.
(Assignee)

Comment 5

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

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/da2b4e883854
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
Target Milestone: --- → mozilla10
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.
(Assignee)

Comment 8

6 years ago
(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.
Depends on: 690828
No longer depends on: 690828
You need to log in before you can comment on or make changes to this bug.