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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Condition for step 2C #90

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

OisinNolan
Copy link
Contributor

@OisinNolan OisinNolan commented Aug 28, 2020

  • This PR broadens the condition for step 2E such that it applies to controls embedded in nodes that allow name from content as well as controls that are embedded in labels.
  • Embedded in label and Embedded in node that allows name from content may then be merged and simplified to descendant of node whose accessible name / description is being computed, as is done in the PR.

This PR is motivated by the fact that many (all?) AccName implementations interpret step 2E in this manner. See example:
image


馃挜 Error: 500 Internal Server Error 馃挜

PR Preview failed to build. (Last tried on May 16, 2021, 3:07 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

馃毃 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

馃敆 Related URL


馃槶  Sorry, there was an error generating the HTML. Please report this issue!
Specification: https://rawcdn.githack.com/OisinNolan/accname/beeb3d59364035373d642da35b6a6dc4e3bf323d/index.html?isPreview=true&publishDate=2021-05-16
ReSpec version: 25.6.0
File a bug: https://github.com/w3c/respec/
Error: Error: Evaluation failed: Timeout: document.respec.ready didn't resolve in 27421ms.
    at ExecutionContext._evaluateInternal (/u/spec-generator/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:221:19)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:110:16)
    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:276:12)
    at async toHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:92:18)
    at async Object.generate [as respec] (/u/spec-generator/generators/respec.js:14:44)
    at async /u/spec-generator/server.js:205:44

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.


Preview | Diff

@jnurthen jnurthen requested a review from accdc September 3, 2020 17:17
@MelSumner MelSumner changed the title Update to Condition for step 2E Update to Condition for step 2C May 16, 2021
@MelSumner MelSumner self-requested a review May 16, 2021 15:07
@MelSumner
Copy link
Contributor

@jnurthen please add to next week's agenda.

@jnurthen
Copy link
Member

Next meeting is on 27 may. No meeting this week due to GAAD.

@MelSumner MelSumner removed the Agenda label Jun 9, 2021
index.html Outdated Show resolved Hide resolved
remove extra space
index.html Outdated Show resolved Hide resolved
Remove blank line added in merge conflict resolution
index.html Outdated Show resolved Hide resolved
remove extra space added in merge conflict resolution
<ul>
<li>If the embedded control has role <a class="role-reference" href="#textbox">textbox</a>, return its value.</li>
<li>If the embedded control has role menu <a class="role-reference" href="#button">button</a>, return the text alternative of the button.</li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<li>If the embedded control has role menu <a class="role-reference" href="#button">button</a>, return the text alternative of the button.</li>
<li>If the embedded control has role <a class="role-reference" href="#menu">menu</a>, return the text alternative of the button that controls the menu.</li>

Since there's no role "menu button" I think this sentence might be a bit more accurate. WDYT?

@MelSumner
Copy link
Contributor

Also @accdc I think I have resolved the merge conflicts so this is probably ready for you to review. 馃憤

Copy link
Contributor

@accdc accdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. :)

@@ -305,9 +305,10 @@ <h4>Computation steps</h4>
</pre>
</details></div>
</li>
<li id="step2C">Otherwise, if the <code>current node</code> is a control embedded within the label (e.g. the <code>label</code> element in HTML or any element directly referenced by <code>aria-labelledby</code>) for another <a class="termref">widget</a>, where the user can adjust the embedded control's value, then return the embedded control as part of the text alternative in the following manner:
<li id="step2C">Otherwise, if the <code>current node</code> is an embedded control whose value can be adjusted by the user, and the <code>current node</code> is a descendant of an element whose <a class="termref internalDFN" href="#dfn-accessible-name" data-link-type="dfn">Accessible Name</a> or <a class="termref internalDFN" href="#dfn-accessible-description" data-link-type="dfn">Accessible Description</a> is being computed, then include the embedded control as part of the text alternative in the following manner:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding whose value can be adjusted would remove readonly controls from the embedded control portion of the accname algo... I think this clause should be removed.

@accdc
Copy link
Contributor

accdc commented Oct 11, 2023

I agree, The text "whose value can be adjusted by the user" should be removed, and simply refer to control which will include all such variations.

@jnurthen
Copy link
Member

jnurthen commented Nov 3, 2023

How does this PR relate to #183 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants