Closed Bug 615180 Opened 9 years ago Closed 9 years ago

introduce ARIARole() method

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
No description provided.
Attachment #493654 - Flags: review?(fherrera)
Attachment #493654 - Flags: approval2.0?
Comment on attachment 493654 [details] [diff] [review]
patch

Drive by comment.

I see you are in 'clean up' mode :)

>+nsAccessible::ARIARoleInternal()
> {
>-  // No ARIA role or it doesn't suppress role from native markup.
>-  if (!mRoleMapEntry || mRoleMapEntry->roleRule != kUseMapRole)
>-    return NativeRole();
>+  NS_PRECONDITION(mRoleMapEntry && mRoleMapEntry->roleRule == kUseMapRole,
>+                  "ARIARoleInternal must be called when ARIA role is applied!");

How about "ARIARoleInternal should only be called when ARIA role overrides!"?
(In reply to comment #1)

> I see you are in 'clean up' mode :)

Yes, just preparations to ongoing big work to get rid worst things while I spot them.

> How about "ARIARoleInternal should only be called when ARIA role overrides!"?

fine with me, thanks.
Attached patch patch2Splinter Review
fix bustage
Attachment #493654 - Attachment is obsolete: true
Attachment #493695 - Flags: review?(fherrera)
Attachment #493695 - Flags: approval2.0?
Attachment #493654 - Flags: review?(fherrera)
Attachment #493654 - Flags: approval2.0?
what patches is this patch depending on? The one at bug #614829 and anything else? I have problems compiling it when applied to trunk + patch at 614829
(In reply to comment #4)
> what patches is this patch depending on? The one at bug #614829 and anything
> else? I have problems compiling it when applied to trunk + patch at 614829

Yep, only bug 614829. Would you file compilation errors? (I didn't get anything from try server).
Nah was my fault fixing a reject at nsDocAccessible.cpp because a line break between 
roleMapEntry && roleMapEntry->role
Comment on attachment 493695 [details] [diff] [review]
patch2

>     if (aIsInsert) {
>-      const nsRoleMapEntry* roleMapEntry = accessible->GetRoleMapEntry();
>-      if (roleMapEntry) {
>-        if (roleMapEntry->role == nsIAccessibleRole::ROLE_MENUPOPUP) {
>-          // Fire EVENT_MENUPOPUP_START if ARIA menu appears.
>-          FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_START,
>-                                     node, AccEvent::eRemoveDupes, aFromUserInput);
>+      if (accessible->ARIARole() == nsIAccessibleRole::ROLE_MENUPOPUP) {
>+        // Fire EVENT_MENUPOPUP_START if ARIA menu appears.
>+        FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_START,
>+                                   node, AccEvent::eRemoveDupes, aFromUserInput);
> 
>-        } else if (roleMapEntry->role == nsIAccessibleRole::ROLE_ALERT) {
>-          // Fire EVENT_ALERT if ARIA alert appears.
>-          updateFlags = eAlertAccessible;
>-          FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_ALERT, node,
>-                                     AccEvent::eRemoveDupes, aFromUserInput);
>-        }
>+      } else if (accessible->ARIARole() == nsIAccessibleRole::ROLE_ALERT) {
>+        // Fire EVENT_ALERT if ARIA alert appears.
>+        updateFlags = eAlertAccessible;
>+        FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_ALERT, node,
>+                                   AccEvent::eRemoveDupes, aFromUserInput);
>       }
> 

We could store here the result of accessible->ARIARole() to save up a function call, although it should not make any noticeable difference. 

Anyway, it looks ok: r=me.
Attachment #493695 - Flags: review?(fherrera) → review+
(In reply to comment #7)

> We could store here the result of accessible->ARIARole() to save up a function
> call, although it should not make any noticeable difference. 

fine with me, thanks
Attachment #493695 - Flags: approval2.0? → approval2.0+
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/e6a435a52576
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.