Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2557713003:
Don't consider large static text nodes as objects (Closed)
|
Created:
4 years ago by David Tseng Modified:
4 years ago Reviewers:
dmazzoni CC:
chromium-reviews, alemate+watch_chromium.org, oshima+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, arv+watch_chromium.org, dtseng+watch_chromium.org, dmazzoni+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't consider large static text nodes as objects
Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes.
This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues.
BUG=671448
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/6c521a1562fa814a048643e1cb886eae9590db09
Cr-Commit-Position: refs/heads/master@{#437405}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Don't consider large static text ndoes as objects #
Messages
Total messages: 22 (12 generated)
Description was changed from ========== Don't consider large static text ndoes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG= ========== to ========== Don't consider large static text ndoes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Don't consider large static text ndoes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Don't consider large static text nodes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG=671448 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dtseng@chromium.org changed reviewers: + dmazzoni@chromium.org
lgtm with nits https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js (right): https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js:29: * http://www.fullondesign.co.uk/design/usability/ This link doesn't work for me. I tried appending the line below this one, it just redirects no matter what I do. https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js:36: constants.MAX_CHARCOUNT = 1500; How about constants.LEAF_MAX_CHARCOUNT just so it's more clear that this is the max to be considered a leaf, not a hard max or something like that BTW, I'd consider something significantly smaller like 300. I think navigating by line should be the default for anything more than a couple of lines worth. Basically I'd set this to be longer than the typical heading or list item, so anything longer than that is considered a paragraph. Up to you though. If you've tried it out and this makes sense to you that's great.
https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js (right): https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js:29: * http://www.fullondesign.co.uk/design/usability/ On 2016/12/07 06:40:51, dmazzoni wrote: > This link doesn't work for me. I tried appending the line below this one, it > just redirects no matter what I do. Removed. https://codereview.chromium.org/2557713003/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/cvox2/background/constants.js:36: constants.MAX_CHARCOUNT = 1500; On 2016/12/07 06:40:50, dmazzoni wrote: > How about constants.LEAF_MAX_CHARCOUNT just so it's more clear that this is the > max to be considered a leaf, not a hard max or something like that > > BTW, I'd consider something significantly smaller like 300. I think navigating > by line should be the default for anything more than a couple of lines worth. > Basically I'd set this to be longer than the typical heading or list item, so > anything longer than that is considered a paragraph. > > Up to you though. If you've tried it out and this makes sense to you that's > great. This was taken pretty much verbatim from group util.js (cClassic). It worked pretty well there so I'm not too inclined to change it. This cl doesn't aim to get the right amount of text to listen to at one time, but to prevent tts from choking on a huge utterance. We're also incurring significant overhead in those cases by having the speech queue in js.
The CQ bit was checked by dtseng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org Link to the patchset: https://codereview.chromium.org/2557713003/#ps20001 (title: "Don't consider large static text ndoes as objects")
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: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
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: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
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": 20001, "attempt_start_ts": 1481239706201870,
"parent_rev": "3a9b23c8692b7ff51f77d29bb84e8d79e5941f5f", "commit_rev":
"88c08792d4d33b9fd6b1de293fff8cbea3cae224"}
Message was sent while issue was closed.
Description was changed from ========== Don't consider large static text nodes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG=671448 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Don't consider large static text nodes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG=671448 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Don't consider large static text nodes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG=671448 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Don't consider large static text nodes as objects Static text nodes > 1500 characters should not be visited during object navigation. Rather, ChromeVox should descend into their inline textboxes. This makes users who want to navigate large text documents loaded into Chrome by either line or object not encounter issues. BUG=671448 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/6c521a1562fa814a048643e1cb886eae9590db09 Cr-Commit-Position: refs/heads/master@{#437405} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/6c521a1562fa814a048643e1cb886eae9590db09 Cr-Commit-Position: refs/heads/master@{#437405} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
