Closed Bug 1251712 Opened 4 years ago Closed 4 years ago

propagate a context flag for alerts

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: surkov, Assigned: surkov)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
No description provided.
Attachment #8724178 - Flags: review?(dbolter)
Before I review can you tell me the short story of 'why' this change?
Flags: needinfo?(surkov.alexander)
(In reply to David Bolter [:davidb] from comment #1)
> Before I review can you tell me the short story of 'why' this change?

this reduce a need to run the tree up on every insertion, that should help for a case when you make lot of insertions deep in the tree
Flags: needinfo?(surkov.alexander)
(In reply to alexander :surkov from comment #2)
> (In reply to David Bolter [:davidb] from comment #1)
> > Before I review can you tell me the short story of 'why' this change?
> 
> this reduce a need to run the tree up on every insertion, that should help
> for a case when you make lot of insertions deep in the tree

Thanks! :)
Comment on attachment 8724178 [details] [diff] [review]
patch

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

r=me, with questions:

::: accessible/generic/Accessible.cpp
@@ +1984,5 @@
>      SetARIAHidden(true);
> +
> +  mContextFlags |=
> +    static_cast<uint32_t>((mParent->IsAlert() ||
> +                           mParent->IsInsideAlert())) & eInsideAlert;

I assume the boolean gets converted to 0xFFF... for the bitwise & operation? (Oh C++ sometimes you smell funny)

::: accessible/generic/Accessible.h
@@ +1148,5 @@
>    int32_t mIndexInParent;
>  
>    static const uint8_t kChildrenFlagsBits = 2;
>    static const uint8_t kStateFlagsBits = 11;
> +  static const uint8_t kContextFlagsBits = 3;

Aside: I don't remember why we later initialize mContextFlags to kContextFlagsBits and not 0?
Attachment #8724178 - Flags: review?(dbolter) → review+
(In reply to David Bolter [:davidb] from comment #5)
> I assume the boolean gets converted to 0xFFF... for the bitwise & operation?

Or 0x000 of course.
(In reply to David Bolter [:davidb] from comment #5)

> >    static const uint8_t kChildrenFlagsBits = 2;
> >    static const uint8_t kStateFlagsBits = 11;
> > +  static const uint8_t kContextFlagsBits = 3;
> 
> Aside: I don't remember why we later initialize mContextFlags to
> kContextFlagsBits and not 0?

it's a memory size
https://hg.mozilla.org/mozilla-central/rev/5ff87fac5346
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Assignee: nobody → surkov.alexander
You need to log in before you can comment on or make changes to this bug.