If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add value and attributes caching tests.

RESOLVED FIXED in Firefox 50

Status

()

Core
Disability Access APIs
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

a year ago
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
(Assignee)

Comment 1

a year ago
Created attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

Review commit: https://reviewboard.mozilla.org/r/56604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56604/
Attachment #8758324 - Flags: review?(eitan)
(Assignee)

Comment 2

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=35aafb436cd3
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

https://reviewboard.mozilla.org/r/56604/#review53712

A few questions below. Thanks!

::: accessible/tests/browser/browser_caching_attributes.js:61
(Diff revision 1)
> +  unexpected: {
> +    'container-live': 'polite',
> +    'live': 'polite'
> +  }
> +}, {
> +  desc: '@id attribute changes, attributes should get updated',

Will this reliably pass all the time? Not waiting for an event makes me nervous.

::: accessible/tests/browser/browser_caching_attributes.js:75
(Diff revision 1)
> +  unexpected: {
> +    'container-live': 'polite',
> +    'live': 'polite'
> +  }
> +}, {
> +  desc: '@aria-live sets container-live and live attributes',

Same, no event. How will this remain reliable?

::: accessible/tests/browser/browser_caching_value.js:72
(Diff revision 1)
> +  desc: 'Value should change when @aria-valuetext is updated',
> +  id: 'slider',
> +  attrs: [{
> +    attr: 'aria-valuenow'
> +  }],
> +  expected: ['hey!', 0, 0, 7, 0]

This is an interesting case, valuetext doesn't change, valuenow does, and you don't get an EVENT_VALUE_CHANGE? I would think that is a bug.
Attachment #8758324 - Flags: review?(eitan)
(Assignee)

Comment 4

a year ago
(In reply to Eitan Isaacson [:eeejay] from comment #3)
> Comment on attachment 8758324 [details]
> MozReview Request: Bug 1276972 - adding value and attributes caching tests.
> r=eeejay
>
> ...
>
> ::: accessible/tests/browser/browser_caching_value.js:72
> (Diff revision 1)
> > +  desc: 'Value should change when @aria-valuetext is updated',
> > +  id: 'slider',
> > +  attrs: [{
> > +    attr: 'aria-valuenow'
> > +  }],
> > +  expected: ['hey!', 0, 0, 7, 0]
> 
> This is an interesting case, valuetext doesn't change, valuenow does, and
> you don't get an EVENT_VALUE_CHANGE? I would think that is a bug.

Here value does not change, what does is nsIAccessibleValue::currentValue in cases were accessible supports nsIAccessibleValue interface.
(Assignee)

Comment 5

a year ago
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56604/diff/1-2/
Attachment #8758324 - Attachment description: MozReview Request: Bug 1276972 - adding value and attributes caching tests. r=eeejay → Bug 1276972 - adding value and attributes caching tests.
Attachment #8758324 - Flags: review?(eitan)
(Assignee)

Comment 6

a year ago
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56604/diff/2-3/
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

https://reviewboard.mozilla.org/r/56604/#review54516

::: accessible/tests/browser/browser_caching_attributes.js:68
(Diff revisions 1 - 2)
>    attrs: [{
>      attr: 'aria-live',
>      value: 'polite'
> +  }, {
> +    // Additionally triggering NAME_CHANGE event to make sure there are no
> +    // intermittents.

I think this is even more problematic. The change alters the actual test. I would keep it the way it is (no waitFor).

But more generally, I think it is a bug that there is no event when the numeric value changes if there is a text value.
Attachment #8758324 - Flags: review?(eitan)
(Assignee)

Comment 8

a year ago
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56604/diff/3-4/
Attachment #8758324 - Flags: review?(eitan)
(Assignee)

Comment 9

a year ago
(In reply to Yura Zenevich [:yzen] from comment #8)
> Comment on attachment 8758324 [details]
> Bug 1276972 - adding value and attributes caching tests.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/56604/diff/3-4/

OK, moved things back when there were no events ?
Comment on attachment 8758324 [details]
Bug 1276972 - adding value and attributes caching tests.

https://reviewboard.mozilla.org/r/56604/#review55100
Attachment #8758324 - Flags: review?(eitan) → review+
(Assignee)

Comment 11

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1357e6366168

Comment 12

a year ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b107bbf2a067
adding value and attributes caching tests. r=eeejay

Comment 13

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b107bbf2a067
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.