Closed Bug 1450896 Opened 6 years ago Closed 6 years ago

"Pick accessible from the page" button becomes greyed out only after click

Categories

(DevTools :: Accessibility Tools, defect)

61 Branch
defect
Not set
normal

Tracking

(firefox61 verified)

VERIFIED FIXED
Firefox 61
Tracking Status
firefox61 --- verified

People

(Reporter: roxana.leitan, Assigned: yzen)

References

Details

Attachments

(2 files)

[Affected versions]:
Nightly 61.0a1

[Affected Platforms]:
Ubuntu 16.04 x64, Windows 10 x64

[Prerequisites]:
- Have the latest try build 61.0a1 from (2018-03-27) installed.
- Accessibility is enabled and accessibility tab is opened

[Steps to reproduce]:
1.Click "Turn off Accessibility Features" button

[Expected result]:
Accessibility should be turned off
"Pick accessible from the page" button should become greyed out


[Actual result]:
"Pick accessible from the page" button is clickable and is not grayed out until click

[Note]:
The issue is intermittent
OS: Linux → All
Hardware: Unspecified → All
Hi Roxana, I can't seem to reproduce it at all on my side. Do you think you might find another way to reproduce this more reliably?
Flags: needinfo?(roxana.leitan)
Could you also try this try build and see if the issue goes away: https://treeherder.mozilla.org/#/jobs?repo=try&revision=358183fe9e68841c70670dae149fb4ed3ac6ad0c thanks!
Hi Yura,

I could not reproduce the issue on Windows 10 x64 and Ubuntu 16.04 x64 using the try build from comment 2
Flags: needinfo?(roxana.leitan)
Attached patch 1450896 patchSplinter Review
(In reply to roxana.leitan@softvision.ro from comment #3)
> Hi Yura,
> 
> I could not reproduce the issue on Windows 10 x64 and Ubuntu 16.04 x64 using
> the try build from comment 2

Great, then I have a patch :)

Toolbar buttons are not guaranteed to re-render when button state object is changed. Toolbox controller has to actually change the state.
Attachment #8968264 - Flags: review?(jdescottes)
Comment on attachment 8968264 [details] [diff] [review]
1450896 patch

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

Nice find!

::: devtools/client/accessibility/accessibility-panel.js
@@ +118,5 @@
>    refresh() {
>      this.cancelPicker();
>  
>      if (this.isVisible) {
> +      this._front.on("init", this._toolbox._updatePickerButton);

1/ Could we rename _updatePickerButton to updatePickerButton in toolbox.js to give a hint that this is a public API.

2/ The code assumes that _updatePickerButton is bound to toolbox. It would be nice if the Accessibility panel didn't depend on this implementation detail. Can we define a dedicated callback bound to `this`?
Attachment #8968264 - Flags: review?(jdescottes) → review+
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4868cb6ca4a6
ensure update picker button is re-rendered when a11y service is enabled/disabled. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/4868cb6ca4a6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Verified fixed using the latest Nightly 61.0a1 (2018-04-18) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.