All users were logged out of Bugzilla on October 13th, 2018

propagate a context flag for alerts

RESOLVED FIXED in Firefox 47

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8724178 [details] [diff] [review]
patch
Attachment #8724178 - Flags: review?(dbolter)
Before I review can you tell me the short story of 'why' this change?
Flags: needinfo?(surkov.alexander)
(Assignee)

Comment 2

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

Comment 8

3 years ago
(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

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5ff87fac5346
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Updated

3 years ago
Assignee: nobody → surkov.alexander
You need to log in before you can comment on or make changes to this bug.