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

Issue 852266 link

Starred by 1 user
Status: Fixed
Owner:
█ nek...@chromium.org
Last visit > 30 days ago
Closed: Jul 1
Cc:
█ nek...@chromium.org
dmazz...@chromium.org
aboxhall@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

DCHECK in AXPosition::CreatePositionInTextObject for css content pseudo elements

Project Member Reported by land...@vewd.com, Jun 13 2018 
Version: master
OS: Linux

What steps will reproduce the problem?
(1) Load attached test case in ChromeOs with chromevox enabled

What is the expected result?
No DCHECK hit

What happens instead?
The following DCHECK is hit:
DCHECK(container.GetNode() && container.GetNode()->IsTextNode())
      << "Text positions should be anchored to a text node.";

Text inserted by CSS content does not have a DOM Node and is referred to as pseudo element or anonymous layout object in the code. The related AXInlineTextBox in the accessibility tree will therefor return null from the GetNode method. This is the trigger of the DCHECK
The new code in AXPosition is heavily depending on that the related container has a  non null Node so even if DCHECKS are turned off the position will not be valid.

The usage of this code was introduced by https://chromium-review.googlesource.com/995976 and reverting that patch removes the problem
assert.html
182 bytes View Download

Comment 1 by dmazz...@chromium.org, Jun 18 2018

Labels: -OS-Linux -Pri-3 Pri-1
Owner: nek...@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 25 2018 

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

commit 7ed4cd1f125806d9c01e877bca5645bd10275fb3
Author: Nektarios Paisios <nektar@chromium.org>
Date: Mon Jun 25 00:14:56 2018

Creates an invalid AXRange if start and end positions are invalid instead of DCHECKing

This creates safer code overall.
If start and end AXPositions are invalid for any reason we didn't contemplate, the browser doesn't crash but returns an AXRange that is explicitly marked invalid.
It's also a nice workaround until CSS decorations are supported in AXPosition.

TBR=dmazzoni@chromium.org, aleventhal@chromium.org

Bug:  852266 
Change-Id: I94db8aeda296690a842b674ea27743dc573205d7
Reviewed-on: https://chromium-review.googlesource.com/1112968
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569941}
[modify] https://crrev.com/7ed4cd1f125806d9c01e877bca5645bd10275fb3/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.cc
[modify] https://crrev.com/7ed4cd1f125806d9c01e877bca5645bd10275fb3/third_party/blink/renderer/modules/accessibility/ax_position.h
[modify] https://crrev.com/7ed4cd1f125806d9c01e877bca5645bd10275fb3/third_party/blink/renderer/modules/accessibility/ax_range.cc
[modify] https://crrev.com/7ed4cd1f125806d9c01e877bca5645bd10275fb3/third_party/blink/renderer/modules/accessibility/ax_selection.cc

Comment 4 by nek...@chromium.org, Jul 1

Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 14 

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

commit 950b2be9ff2b5e475da0f2f216bd8d7201380ae2
Author: Nektarios Paisios <nektar@chromium.org>
Date: Tue Aug 14 16:14:22 2018

Completes implementation of AXPosition in Blink

1. Exposes AXPositionAdjustmentBehavior on all constructor methods and propagates adjustment behavior to constructor methods when converting from DOM positions.
2. Add comment documenting default adjustment behavior.
3. When converting from a DOM to an AX position but the corresponding AX position is ignored, adjusts to neighboring DOM positions to minimize huge diviations between the DOM and the corresponding AX position.
E.g. if one cell in a table row is ignored, we don't want to skip to the next table row.
4. Introduces tests for ignored objects, affinity, and HTML labels.
5. Enabled two disabled tests with: list markers and ARIA hidden.
6. Modified |AXObject::Next\PreviousInTree| to return the next / previous valid unignored object in the tree when called on accessibility ignored objects.
7. Audited all call sides for |AXObject::Next\PreviousSibling| and disallowed calling those methods on ignored objects by using |NOTREACHED|.
8. Fixed |CreatePreviousPosition| to handle a corner case: When the previous object is both the previous sibling of the container object and also a text object.
9. Fixed |ToDOMPosition| to return the correct DOM position when the AX position is "after childre" but there are some ignored children at the end of the container object.
10. Fixed |CreateNextPosition| to return an "after children" position when the previous position was inside a text object which is the child of an unignored container.
11. Investigated |TextIterator| to ensure that it doesn't skip line breaks.
12. Fixed the handling of canvas elements:
A) Fallback content can also be static text not only element nodes.
B) JavaScript might not run but we would still want the role to be kCanvasRole, not kGenericContainer.
C) The way that we were adding children caused a DCHECK to trigger because fallback content may also be in the layout tree under some circomstances.

R=dmazzoni@chromium.org

Bug:  852266 
Change-Id: Iedf9f61bdf0b5172605c7268cbb20966f9249c93
Reviewed-on: https://chromium-review.googlesource.com/1150669
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582930}
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/content/test/data/accessibility/html/canvas-expected-android.txt
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/content/test/data/accessibility/html/canvas-expected-blink.txt
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/content/test/data/accessibility/html/canvas-expected-mac.txt
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/content/test/data/accessibility/html/canvas-expected-win.txt
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/content/test/data/accessibility/html/canvas.html
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/WebKit/LayoutTests/accessibility/canvas-description-and-role-expected.txt
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/core/dom/node_traversal.h
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_layout_object.h
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_position.cc
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_position.h
[modify] https://crrev.com/950b2be9ff2b5e475da0f2f216bd8d7201380ae2/third_party/blink/renderer/modules/accessibility/ax_position_test.cc
►

Sign in to add a comment