Fire state change event for aria-busy

VERIFIED FIXED in mozilla6

Status

()

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

People

(Reporter: davidb, Assigned: tbsaunde)

Tracking

({dev-doc-complete})

Trunk
mozilla6
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 524280 [details] [diff] [review]
WIP
(Reporter)

Updated

6 years ago
Assignee: nobody → bolterbugz

Comment 1

6 years ago
David,
1) please provide correct bug name (we have some implementation of aria-busy already)
2) bug description (no idea what we must do here)
3) any chance to finish the patch?

Comment 2

6 years ago
I think this came out of an email thread between Yahoo!, David and I. Since there is a busy state, aria-busy should be exposed using the busy state and a state change event should be fired when it changes.
Blocks: 658181

Comment 3

6 years ago
Thanks, Jamie. Changing bug summary per comment #2.
Summary: Implement aria-busy → Fire state change event for aria-busy
(Assignee)

Comment 4

6 years ago
Created attachment 534701 [details] [diff] [review]
UPDATE PATCH

update patch for trunk and clean up some
Assignee: bolterbugz → trev.saunders
Attachment #524280 - Attachment is obsolete: true
Attachment #534701 - Flags: review?(surkov.alexander)

Comment 5

6 years ago
Comment on attachment 534701 [details] [diff] [review]
UPDATE PATCH

Review of attachment 534701 [details] [diff] [review]:
-----------------------------------------------------------------

too many nits I'd like to look at next patch

::: accessible/src/base/nsDocAccessible.cpp
@@ +1037,5 @@
>      return;
>    }
>  
> +  if (aAttribute == nsAccessibilityAtoms::aria_busy) {
> +    PRBool change = !aContent->AttrValueIs(aNameSpaceID, aAttribute,

isOn, isTrue or something more descriptive than change?

@@ +1038,5 @@
>    }
>  
> +  if (aAttribute == nsAccessibilityAtoms::aria_busy) {
> +    PRBool change = !aContent->AttrValueIs(aNameSpaceID, aAttribute,
> +                                         nsAccessibilityAtoms::_true, eCaseMatters);

nit: indent the second line

@@ +1039,5 @@
>  
> +  if (aAttribute == nsAccessibilityAtoms::aria_busy) {
> +    PRBool change = !aContent->AttrValueIs(aNameSpaceID, aAttribute,
> +                                         nsAccessibilityAtoms::_true, eCaseMatters);
> +    nsAccessible* acc = GetAccService()->GetAccessible(aContent);

do GetAccessible(aContent) simply in arguments (no local variable)

::: accessible/tests/mochitest/events/test_aria_statechange.html
@@ +26,5 @@
>       */
>      var gQueue = null;
>  
> +    // Debug stuff.
> +    gA11yEventDumpID = "eventdump";

nit: keep it commented

@@ +45,4 @@
>                     EXT_STATE_EXPANDABLE);
>        };
>  
> +      this.getID = function expand_getID() {

nit: expandNode_getID() please

@@ +49,5 @@
>          return prettyName(aNodeOrID) + " aria-expanded changed";
>        };
>      }
>  
> +    function busyify(aNodeOrID, bBusy)

funny, hope that comes with derivation in english :)

bBusy is also funny, but let's keep 'a' for this :)

@@ +64,5 @@
> +                   (bBusy ? 0 : STATE_BUSY), 0);
> +      };
> +
> +      this.getID = function busyify_getID() {
> +        return prettyName(aNodeOrID) + " aria-busy changed";

please point how it was changed

@@ +77,5 @@
>        gQueue.push(new expandNode("section", false));
>        gQueue.push(new expandNode("div", true));
>        gQueue.push(new expandNode("div", false));
>  
> +      document.getElementById("aria_doc").focus();

what is that, sounds like artifact from David's patch

@@ +107,2 @@
>    <div id="section" role="section" aria-expanded="false">expandable section</div>
>    <div id="div" aria-expanded="false">expandable native div</div>

somewhere here you should add <a> with reference to the bug.
Attachment #534701 - Flags: review?(surkov.alexander)
(Assignee)

Comment 6

6 years ago
Created attachment 534719 [details] [diff] [review]
FIX NITS

HOPEFULLY i GOT ALL THE NITS :)
Attachment #534701 - Attachment is obsolete: true
Attachment #534719 - Flags: review?(surkov.alexander)

Comment 7

6 years ago
Comment on attachment 534719 [details] [diff] [review]
FIX NITS

Review of attachment 534719 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: accessible/src/base/nsDocAccessible.cpp
@@ +1038,5 @@
>    }
>  
> +  if (aAttribute == nsAccessibilityAtoms::aria_busy) {
> +    PRBool isOn = !aContent->AttrValueIs(aNameSpaceID, aAttribute,
> +                                           nsAccessibilityAtoms::_true, eCaseMatters);

it'd be great to have right identation :)
Attachment #534719 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 8

6 years ago
Created attachment 534746 [details] [diff] [review]
PATCH TO LAND

INDENTATION SHOULD BE CORRECT NOW
Attachment #534719 - Attachment is obsolete: true

Comment 9

6 years ago
Landed on mozilla-central on Trevor's behalf: http://hg.mozilla.org/mozilla-central/rev/17d9511255e9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Version: unspecified → Trunk
Keywords: dev-doc-needed
The docs already indicate this is what's supposed to happen, so I'm listing it as a bug fix on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete

Comment 11

6 years ago
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0) Gecko/20100101 Firefox/6.0

All the proposed, reviewed and landed changes in m-c are visible in hg, under beta (i.e. test_aria_changeset.html in http://hg.mozilla.org/releases/mozilla-beta/file/f3e82fad65b2/accessible/tests/mochitest/events )
Setting this as Verified for Firefox 6 Beta.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.