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.

fix bustage

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.

(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

landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/e6a435a52576