New issue
Advanced search Search tips

Issue 918536 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 14
Components:
EstimatedDays: ----
NextAction: 2019-01-14
OS: Chrome
Pri: 1
Type: Bug-Regression
Team-Accessibility



Sign in to add a comment

copy paste with chromevox are broken. e.g press chromevox+s to start marking. you cant use skift to mark text.

Reported by mjonsson...@gmail.com, Jan 2

Issue description

Version: 73.0.3644.0
Reproduction Steps: 
1.
2.
3.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c2cf078f3c913160087e3417d666cdbdb89a1a5

commit 7c2cf078f3c913160087e3417d666cdbdb89a1a5
Author: David Tseng <dtseng@chromium.org>
Date: Fri Jan 04 20:26:44 2019

Fix exception in automation js bindings

The following stack originates from automation_node.js (within the automation api js internal bindings):
Error: Error in event handler for locationChanged during phase 1: ReferenceError: RoleType is not defined
    at Output.subNode_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:899:66)
    at Output.render_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:799:6)
    at Output.withLocation (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:779:207)
    at DesktopAutomationHandler.onLocationChanged (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1546:138)
    at DesktopAutomationHandler.<anonymous> (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1295:8)

The issue is that |RoleType| is used within |boundsForRange|, but it isn't actually defined anywhere.

The internal bindings do not have access to chrome.automation (since the public bindings are defined later within the page context of the extension).

Since the C++ bindings already check for inlineTextBox, clean up this logic.

This does remove the behavior where |boundsForRange| when passed a non-inlineTextBox, would simply return the node's global bounds. This appears to have been the original intention.

Bug:  918536 
Test: manually verify ChromeVox works again when navigating by character (which calls boundsForRange).
Change-Id: I9d77f8d4fe7e8cd6afde2eb9a9b9e1b40aef1d72
Reviewed-on: https://chromium-review.googlesource.com/c/1394793
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620048}
[modify] https://crrev.com/7c2cf078f3c913160087e3417d666cdbdb89a1a5/chrome/renderer/extensions/automation_internal_custom_bindings.cc
[modify] https://crrev.com/7c2cf078f3c913160087e3417d666cdbdb89a1a5/chrome/renderer/resources/extensions/automation/automation_node.js

Labels: -Type-Bug -Pri-2 Merge-Request-72 M-72 Pri-1 Type-Bug-Regression
NextAction: 2019-01-14
Status: Started (was: Unconfirmed)
Project Member

Comment 3 by sheriffbot@chromium.org, Jan 14

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
 Issue 920969  has been merged into this issue.
 Issue 919713  has been merged into this issue.
Labels: -Merge-Review-72 Merge-Approved-72
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 14

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/080a2b758c56e542814ea8ab1ec2e40f0bbd2c58

commit 080a2b758c56e542814ea8ab1ec2e40f0bbd2c58
Author: David Tseng <dtseng@chromium.org>
Date: Mon Jan 14 18:57:29 2019

Merge to m72: Fix exception in automation js bindings

TBR=dtseng@chromium.org
The following stack originates from automation_node.js (within the automation api js internal bindings):
Error: Error in event handler for locationChanged during phase 1: ReferenceError: RoleType is not defined
    at Output.subNode_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:899:66)
    at Output.render_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:799:6)
    at Output.withLocation (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:779:207)
    at DesktopAutomationHandler.onLocationChanged (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1546:138)
    at DesktopAutomationHandler.<anonymous> (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1295:8)

The issue is that |RoleType| is used within |boundsForRange|, but it isn't actually defined anywhere.

The internal bindings do not have access to chrome.automation (since the public bindings are defined later within the page context of the extension).

Since the C++ bindings already check for inlineTextBox, clean up this logic.

This does remove the behavior where |boundsForRange| when passed a non-inlineTextBox, would simply return the node's global bounds. This appears to have been the original intention.

Bug:  918536 
Test: manually verify ChromeVox works again when navigating by character (which calls boundsForRange).
Change-Id: I9d77f8d4fe7e8cd6afde2eb9a9b9e1b40aef1d72
Reviewed-on: https://chromium-review.googlesource.com/c/1394793
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620048}(cherry picked from commit 7c2cf078f3c913160087e3417d666cdbdb89a1a5)
Reviewed-on: https://chromium-review.googlesource.com/c/1409697
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#670}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/080a2b758c56e542814ea8ab1ec2e40f0bbd2c58/chrome/renderer/extensions/automation_internal_custom_bindings.cc
[modify] https://crrev.com/080a2b758c56e542814ea8ab1ec2e40f0bbd2c58/chrome/renderer/resources/extensions/automation/automation_node.js

Status: Fixed (was: Started)
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 080a2b758c56e542814ea8ab1ec2e40f0bbd2c58 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/080a2b758c56e542814ea8ab1ec2e40f0bbd2c58

Commit: 080a2b758c56e542814ea8ab1ec2e40f0bbd2c58
Author: dtseng@chromium.org
Commiter: dtseng@chromium.org
Date: 2019-01-14 18:57:29 +0000 UTC

Merge to m72: Fix exception in automation js bindings

TBR=dtseng@chromium.org
The following stack originates from automation_node.js (within the automation api js internal bindings):
Error: Error in event handler for locationChanged during phase 1: ReferenceError: RoleType is not defined
    at Output.subNode_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:899:66)
    at Output.render_ (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:799:6)
    at Output.withLocation (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:779:207)
    at DesktopAutomationHandler.onLocationChanged (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1546:138)
    at DesktopAutomationHandler.<anonymous> (chrome-extension://mndnfokpggljbaajbnioimlmbfngpief/chromeVox2ChromeBackgroundScript.js:1295:8)

The issue is that |RoleType| is used within |boundsForRange|, but it isn't actually defined anywhere.

The internal bindings do not have access to chrome.automation (since the public bindings are defined later within the page context of the extension).

Since the C++ bindings already check for inlineTextBox, clean up this logic.

This does remove the behavior where |boundsForRange| when passed a non-inlineTextBox, would simply return the node's global bounds. This appears to have been the original intention.

Bug:  918536 
Test: manually verify ChromeVox works again when navigating by character (which calls boundsForRange).
Change-Id: I9d77f8d4fe7e8cd6afde2eb9a9b9e1b40aef1d72
Reviewed-on: https://chromium-review.googlesource.com/c/1394793
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620048}(cherry picked from commit 7c2cf078f3c913160087e3417d666cdbdb89a1a5)
Reviewed-on: https://chromium-review.googlesource.com/c/1409697
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#670}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment