New issue
Advanced search Search tips

Issue 843746 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 19
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Accessibility nextOnLine and previousOnLine are sometimes out of date

Project Member Reported by dmazz...@chromium.org, May 16 2018

Issue description

You're supposed to be able to walk all of the nodes on one line by following nextOnLine and previousOnLine, but sometimes when an edit happens in the middle of a paragraph, for example, they'll get out of date.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 18

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

commit c101cb728aecdd2bb77d29ebe8deaee47261f65e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Jul 18 23:22:32 2018

Accessibility should always serialize an entire block when something changes.

Whenever we post a change to the accessibility tree affecting an element,
if that element isn't block-level we should walk up to the nearest block-level
element and serialize the whole thing.

The reason is that we have fields like nextOnLine and previousOnLine that
depend on the line layout, which can change when changes happen to inline
elements.

Eventually I'd like to have a better concept of marking part of the
accessibility tree "dirty" as a way of cleaning up this logic, but
in the meantime this is hopefully a good fix with a test that can
potentially help fix some real bugs now.

Bug:  843746 , 651614
Change-Id: I411a13c63b0c88ab225393b5287cb87a5887565b
Reviewed-on: https://chromium-review.googlesource.com/1062661
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576249}
[add] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/browser/accessibility/line_layout_browsertest.cc
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/renderer/accessibility/render_accessibility_impl.cc
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/BUILD.gn
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/aria-combo-box-next-expected-win.txt
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/listbox-next-expected-win.txt
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/menulist-collapse-expected-mac.txt
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/menulist-collapse-expected-win.txt
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/menulist-focus-expected-mac.txt
[modify] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/event/menulist-focus-expected-win.txt
[add] https://crrev.com/c101cb728aecdd2bb77d29ebe8deaee47261f65e/content/test/data/accessibility/lines/lines.html

Status: Fixed (was: Started)

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 15

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

commit b53fffcc225fd0f37ab7f56ba9f075f319d5e439
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Aug 15 06:49:41 2018

Roll back code that serialized at the CSS block level due to performance.

crrev.com/c/1062661 made it so that any time an accessibility node changes,
we re-serialize the entire CSS block it's contained in - while this fixed
some correctness issues, it introduced a performance regression that's
too significant to ignore.

This change rolls back the fix for now until we can find a way to fix it
that doesn't impact performance.

TBR=dtseng@chromium.org

Bug:  868830 , 865621,  843746 
Change-Id: I900a4e6ba796608974e881f091b99b0d45c09536
Reviewed-on: https://chromium-review.googlesource.com/1174851
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583182}
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/browser/accessibility/line_layout_browsertest.cc
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/renderer/accessibility/render_accessibility_impl.cc
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/aria-combo-box-next-expected-win.txt
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/listbox-next-expected-win.txt
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/menulist-collapse-expected-mac.txt
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/menulist-collapse-expected-win.txt
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/menulist-focus-expected-mac.txt
[modify] https://crrev.com/b53fffcc225fd0f37ab7f56ba9f075f319d5e439/content/test/data/accessibility/event/menulist-focus-expected-win.txt

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c376201983d79d88a1650b108a83888853404d3

commit 5c376201983d79d88a1650b108a83888853404d3
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Fri Aug 17 18:10:49 2018

Merge to M69: Roll back code that serialized at the CSS block level due to performance.

crrev.com/c/1062661 made it so that any time an accessibility node changes,
we re-serialize the entire CSS block it's contained in - while this fixed
some correctness issues, it introduced a performance regression that's
too significant to ignore.

This change rolls back the fix for now until we can find a way to fix it
that doesn't impact performance.

TBR=dtseng@chromium.org

Bug:  868830 , 865621,  843746 
Change-Id: I900a4e6ba796608974e881f091b99b0d45c09536
Reviewed-on: https://chromium-review.googlesource.com/1174851
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583182}(cherry picked from commit b53fffcc225fd0f37ab7f56ba9f075f319d5e439)
Reviewed-on: https://chromium-review.googlesource.com/1180141
Cr-Commit-Position: refs/branch-heads/3497@{#691}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/browser/accessibility/line_layout_browsertest.cc
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/renderer/accessibility/render_accessibility_impl.cc
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/aria-combo-box-next-expected-win.txt
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/listbox-next-expected-win.txt
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/menulist-collapse-expected-mac.txt
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/menulist-collapse-expected-win.txt
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/menulist-focus-expected-mac.txt
[modify] https://crrev.com/5c376201983d79d88a1650b108a83888853404d3/content/test/data/accessibility/event/menulist-focus-expected-win.txt

Sign in to add a comment