New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644805 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug
Team-Accessibility

Blocked on:
issue 857177



Sign in to add a comment

Aria label does not have a sensible max size

Project Member Reported by dtseng@chromium.org, Sep 7 2016

Issue description

Load:

<div id="big"></div>
<script>
  var huge = document.getElementById('big');
  var count = 1000000;
  var bigStr = '';
  for (var i = 0; i < count; i++)
    bigStr += 'x';
  huge.setAttribute('aria-label', bigStr);
</script>

result:
The label comes through, untruncated. Pushing the value of |count| up causes the browser to hang.

expected:
truncate at a reasonable length; perhaps a factor of the size of the element.

 
I have seen Safari's WebKit impose limits like that in some instances, so perhaps we should use similar limits if present.
Labels: Hotlist-GoodFirstBug
Seeking clarification and repro steps.

Doug and I tried to reproduce this bug as follows:
We ran with and without --force-renderer-accessibility, and also tried changing to a nonsense attribute name. In all cases, we got the same results.
The results were: no hang unless we increased count by a factor of 1000.

In other words, we see no discrepancy between random attributes and ARIA, and whether accessibility is enabled.

Another concern is whether we should be addressing size limits one attribute at a time. What about alt, title, etc.?
Perhaps we need to run with a screen reader? It might be something that screen readers need to do on their end if they are experiencing performance issues with long labels.
I remember that Safari places a limit on aria-labels but I think the code to do this is in the Mac accessibility section, not in the generic DOM parsing section of WebKit.
It's probably easier to hang with ChromeVox.

I think it'd probably make sense to have limits for all attributes that return string values.

My thought was to add something for aria-label, then generalize it to other attributes.

Labels: NewComponent-Accessibility-Blink
Labels: NewComponent-Accessibility
Components: Blink>Accessibility
Components: -UI>Accessibility
Labels: -newcomponent-accessibility-blink -newcomponent-accessibility
Labels: triage-alice
Labels: -triage-alice
Labels: triage-dtseng
Labels: -triage-dtseng
Owner: jamwalla@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 27 2018

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

commit db409e207be0537d47746ab501420946adc48506
Author: James Wallace-Lee <jamwalla@chromium.org>
Date: Wed Jun 27 17:05:00 2018

Truncate string attributes in the accessibility tree

Adds the helper function
BlinkAXTreeSource::TruncateAndAddStringAttribute, which truncates string
attributes when Blink's tree is serialized for accessibility. String
attributes are truncated at BlinkAXTreeSource::kMaxStringAttributeLength
bytes, currently set to 10000 bytes.

Bug:  644805 
Change-Id: Ibd1af03a77814aa524dcb067ad1be3f3f4291c8b
Reviewed-on: https://chromium-review.googlesource.com/1113946
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: James Wallace-Lee <jamwalla@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570818}
[modify] https://crrev.com/db409e207be0537d47746ab501420946adc48506/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/db409e207be0537d47746ab501420946adc48506/content/renderer/accessibility/blink_ax_tree_source.cc
[modify] https://crrev.com/db409e207be0537d47746ab501420946adc48506/content/renderer/accessibility/blink_ax_tree_source.h
[add] https://crrev.com/db409e207be0537d47746ab501420946adc48506/content/test/data/accessibility/html/truncate-label-expected-blink.txt
[add] https://crrev.com/db409e207be0537d47746ab501420946adc48506/content/test/data/accessibility/html/truncate-label.html

Status: Fixed (was: Assigned)
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 27 2018

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

commit b767d54f17fc695549bd409d323e3f0b86392cbd
Author: Peter Mayo <petermayo@chromium.org>
Date: Wed Jun 27 19:34:33 2018

Speculative: failure on Fuschia x64 reported.

Revert "Truncate string attributes in the accessibility tree"

This reverts commit db409e207be0537d47746ab501420946adc48506.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Truncate string attributes in the accessibility tree
> 
> Adds the helper function
> BlinkAXTreeSource::TruncateAndAddStringAttribute, which truncates string
> attributes when Blink's tree is serialized for accessibility. String
> attributes are truncated at BlinkAXTreeSource::kMaxStringAttributeLength
> bytes, currently set to 10000 bytes.
> 
> Bug:  644805 
> Change-Id: Ibd1af03a77814aa524dcb067ad1be3f3f4291c8b
> Reviewed-on: https://chromium-review.googlesource.com/1113946
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Commit-Queue: James Wallace-Lee <jamwalla@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570818}

TBR=dmazzoni@chromium.org,dtseng@chromium.org,jamwalla@chromium.org

Change-Id: Id8d54a3a1767d8fa75f54a9c0d954c44c376018f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  644805 
Reviewed-on: https://chromium-review.googlesource.com/1117222
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570858}
[modify] https://crrev.com/b767d54f17fc695549bd409d323e3f0b86392cbd/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/b767d54f17fc695549bd409d323e3f0b86392cbd/content/renderer/accessibility/blink_ax_tree_source.cc
[modify] https://crrev.com/b767d54f17fc695549bd409d323e3f0b86392cbd/content/renderer/accessibility/blink_ax_tree_source.h
[delete] https://crrev.com/0a301aa216782ecb93a9de448eea245106a65be8/content/test/data/accessibility/html/truncate-label-expected-blink.txt
[delete] https://crrev.com/0a301aa216782ecb93a9de448eea245106a65be8/content/test/data/accessibility/html/truncate-label.html

Blockedon: 857177
Status: Assigned (was: Fixed)
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 9

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

commit 6b27a5d6665f01a601069b67c0ee0b429c71310d
Author: James Wallace-Lee <jamwalla@chromium.org>
Date: Mon Jul 09 19:32:07 2018

Reland "Truncate string attributes in the accessibility tree"

This is a reland of db409e207be0537d47746ab501420946adc48506

Original change's description:
> Truncate string attributes in the accessibility tree
> 
> Adds the helper function
> BlinkAXTreeSource::TruncateAndAddStringAttribute, which truncates string
> attributes when Blink's tree is serialized for accessibility. String
> attributes are truncated at BlinkAXTreeSource::kMaxStringAttributeLength
> bytes, currently set to 10000 bytes.
> 
> Bug:  644805 
> Change-Id: Ibd1af03a77814aa524dcb067ad1be3f3f4291c8b
> Reviewed-on: https://chromium-review.googlesource.com/1113946
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Commit-Queue: James Wallace-Lee <jamwalla@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570818}

Bug:  644805 
Change-Id: If084733234602882bbd37fc1891e49011b12ada8
Reviewed-on: https://chromium-review.googlesource.com/1129399
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: James Wallace-Lee <jamwalla@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573406}
[modify] https://crrev.com/6b27a5d6665f01a601069b67c0ee0b429c71310d/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/6b27a5d6665f01a601069b67c0ee0b429c71310d/content/renderer/accessibility/blink_ax_tree_source.cc
[modify] https://crrev.com/6b27a5d6665f01a601069b67c0ee0b429c71310d/content/renderer/accessibility/blink_ax_tree_source.h
[add] https://crrev.com/6b27a5d6665f01a601069b67c0ee0b429c71310d/content/test/data/accessibility/html/truncate-label-expected-blink.txt
[add] https://crrev.com/6b27a5d6665f01a601069b67c0ee0b429c71310d/content/test/data/accessibility/html/truncate-label.html

Status: Fixed (was: Assigned)
Relanded and the fuschia_x64 bot looks good, so closing again.
This is causing ascii pages to get truncated.
See  issue 896359 .

Sign in to add a comment