Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shadow DOM issue for color contrast rule #687

Closed
marcysutton opened this issue Jan 19, 2018 · 4 comments
Closed

Shadow DOM issue for color contrast rule #687

marcysutton opened this issue Jan 19, 2018 · 4 comments
Labels
color contrast Color contrast issues

Comments

@marcysutton
Copy link
Contributor

axe-core is having trouble calculating color contrast inside of Shadow DOM, and it puts items into Needs Review for some reason. I've seen it occur for implicit labels with visually hidden text as well as buttons with visible text. I tested this with the latest dev build of aXe Coconut.

I believe @robdodson saw this issue a while back, too.

Example markup to populate within a shadow boundary (from my a11y-demo-app, uses React-Shadow):

<trip-planner>
        <ShadowDOM include={[
          'css/normalize.css',
          '/fonts/font-awesome-4.2.0/css/font-awesome.min.css',
          '/css/trip-planner.css']
        }>
          <div>
            <form>
              <div>
                <div className="button-wrap left">
                  <button type="button" title="My trips">
                    <span class="visuallyhidden">My trips</span>
                    <i className="fa fa-star"></i>
                  </button>
                </div>
                <label className="fit">
                  <span class="visuallyhidden">Name of trip</span>
                  <input type="text" placeholder="Name of trip" />
                </label>
                <label className="fit">
                  <span class="visuallyhidden">Name of ski area</span>
                  <input type="text" placeholder="Name of ski area" />
                </label>
                <div className="button-wrap">
                  <button type="button" className="expand" title="Expand">
                    <span class="visuallyhidden">Expand widget</span>
                    <i className="fa fa-caret-down"></i>
                  </button>
                </div>
              </div>
              <div>
                <div className="inputGroup left">
                  <button type="button" aria-label="Select a previous date">
                    <i className="fa fa-caret-left"></i>
                  </button>
                  <label>
                    <span class="visuallyhidden">Date of trip</span>
                    <input type="text" placeholder="16.1.18" />
                  </label>
                  <button type="button" aria-label="Select a later date">
                    <i className="fa fa-caret-right"></i>
                  </button>
                </div>
                <div className="button-wrap right">
                  <button type="button" aria-label="Date picker">
                    <i className="fa fa-calendar"></i>
                  </button>
                </div>
                <div className="select right">
                  <label>
                    <span class="visuallyhidden">type of trip</span>
                    <select>
                      <option>Type of trip</option>
                      <option>Alpine ski resort</option>
                      <option>Backcountry</option>
                      <option>Nordic</option>
                    </select>
                  </label>
                </div>
                <div className="button-wrap right">
                  <button className="submit">
                    Go
                  </button>
                </div>
              </div>
            </form>
          </div>
        </ShadowDOM>
      </trip-planner>

review item for GO button

inspecting GO button in devtools

@marcysutton marcysutton added color contrast Color contrast issues aXe 3.0 labels Jan 19, 2018
@robdodson
Copy link
Contributor

I tried it with Lighthouse on Chrome Canary which should be running a fairly recent version of axe and the only contrast warning I got was on h1.App-title. But trying with axe-coconut does show the contrast warning.

Either way, I tried to put together a smaller repro test case to remove some variables.
https://output.jsbin.com/tuyapagiqe/1

This is just a vanilla custom element with shadow dom using some of the styles from your app. I was able to get axe-coconut to fail on this so it seems like it could be used to debug.

@dylanb
Copy link
Contributor

dylanb commented Jan 19, 2018

The problem here is that color.getRectStack is using document instead of the shadowRoot of the element to find the background elements. It ends up getting only the background elements in the main document which do not contain the actual node. So we need to update that set of functions to be virtual DOM aware.

Essentially instead of simply doing a single call to elementsFromPoint, we will need to traverse up the shadow DOMS and collect all the nodes all the way up to the top document.

@marcysutton
Copy link
Contributor Author

marcysutton commented Jan 23, 2018

@dylanb can't we use axe.commons.dom.getRootNode in place of document? That seems to work in my integration tests.

@marcysutton
Copy link
Contributor Author

And right after I wrote my last comment, I understood why. If a shadow tree is on top of a background color, we won't pick up the entire stack. I'll try combining multiple stack arrays as a first attempt.

marcysutton pushed a commit that referenced this issue Feb 1, 2018
marcysutton pushed a commit that referenced this issue Feb 1, 2018
marcysutton pushed a commit that referenced this issue Feb 1, 2018
marcysutton added a commit that referenced this issue Feb 7, 2018
* chore: move isShadowRoot to its own file

* fix: add shadow dom support to color contrast rule

Closes #687

* test(color): add tests, incorporate feedback

* feat: move shadowElementsFromPoint to commons.dom

* test: upgrade to circle 2.0 and try failing tests again
mrtnvh pushed a commit to mrtnvh/axe-core that referenced this issue Nov 24, 2023
Co-authored-by: Steven Lambert <steven.lambert@deque.com>
Co-authored-by: Dan Bjorge <dan.bjorge@deque.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
color contrast Color contrast issues
Projects
None yet
Development

No branches or pull requests

3 participants