Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2966273002:
Provide complete, smart, spoken feedback for editable selection (Closed)
|
Created:
3 years, 5 months ago by David Tseng Modified:
3 years, 5 months ago Reviewers:
dmazzoni CC:
chromium-reviews, alemate+watch_chromium.org, dtseng+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, dougt+watch_chromium.org, dmazzoni+watch_chromium.org, oshima+watch_chromium.org, aleventhal+watch_chromium.org, je_julie, arv+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionProvide complete, smart, spoken feedback for editable selection
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2966273002
Cr-Commit-Position: refs/heads/master@{#484817}
Committed: https://chromium.googlesource.com/chromium/src/+/2404e07cab907a69ef1c26658734ec7278e4b808
Patch Set 1 #Patch Set 2 : And a test #Patch Set 3 : Even more testing. #Patch Set 4 : A bit more cleanup. #Patch Set 5 : Fix test. #
Total comments: 2
Patch Set 6 : Rename. #
Dependent Patchsets: Messages
Total messages: 27 (19 generated)
Description was changed from ========== Provide complete, smart, spoken feedback for editable selection BUG= ========== to ========== Provide complete, smart, spoken feedback for editable selection BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dtseng@chromium.org changed reviewers: + dmazzoni@chromium.org
Ready for a look. PTAL.
The CQ bit was checked by dtseng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dtseng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2966273002/diff/80001/chrome/browser/resource... File chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js (right): https://codereview.chromium.org/2966273002/diff/80001/chrome/browser/resource... chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:330: // Note that base and extend are used here rather than anchor We should use terminology correctly. Currently Blink is exposing the selection start and end, but that's a bug. This change fixes it so that it really exposes the focus and anchor, as advertised: https://chromium-review.googlesource.com/c/562645/ Inside Blink code, base and extent (not extend) are the same as focus and anchor. I don't think we need to invent different names here, we should just fix the underlying bug and then use focus and anchor consistently.
PTAL https://codereview.chromium.org/2966273002/diff/80001/chrome/browser/resource... File chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js (right): https://codereview.chromium.org/2966273002/diff/80001/chrome/browser/resource... chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js:330: // Note that base and extend are used here rather than anchor On 2017/07/06 21:47:01, dmazzoni wrote: > We should use terminology correctly. Currently Blink > is exposing the selection start and end, but that's a bug. > This change fixes it so that it really exposes the focus > and anchor, as advertised: > > https://chromium-review.googlesource.com/c/562645/ > > Inside Blink code, base and extent (not extend) are > the same as focus and anchor. I don't think we need to > invent different names here, we should just fix the > underlying bug and then use focus and anchor consistently. > I agree but would also like to land this and the dependent Docs change. I've renamed the extend -> extent and added a TODO. Note that this class already tries to detect the anchor (as you've correctly defined) above so it requires another change at least anyway to clean up.
lgtm
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dtseng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1499396994144760,
"parent_rev": "e3dc889102e2834322b6f740096098ba9deb8062", "commit_rev":
"2404e07cab907a69ef1c26658734ec7278e4b808"}
Message was sent while issue was closed.
Description was changed from ========== Provide complete, smart, spoken feedback for editable selection BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Provide complete, smart, spoken feedback for editable selection BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2966273002 Cr-Commit-Position: refs/heads/master@{#484817} Committed: https://chromium.googlesource.com/chromium/src/+/2404e07cab907a69ef1c26658734... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/2404e07cab907a69ef1c26658734... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
