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

Need way to expose content insertions and deletions #4

Closed
aleventhal opened this issue May 21, 2018 · 13 comments
Closed

Need way to expose content insertions and deletions #4

aleventhal opened this issue May 21, 2018 · 13 comments

Comments

@aleventhal
Copy link
Contributor

aleventhal commented May 21, 2018

Content sometimes needs to indicate a content insertion/deletion (can be proposed change to content or has already occurred). In HTML this is indicated with the <ins> and <del> tags. Also, ARIA 1.2 plans to add roles for this. This will be very useful for online collaborative word processors and editors, such as Google Docs.

Mick Curran suggested that this be a text attribute. For example: diff:del and diff:ins. It could also be a role.

I suggest going with Mick's recommendation unless there's a counter proposal.

@jcsteh
Copy link
Collaborator

jcsteh commented May 22, 2018

I agree with this being a text attribute. I'd suggest different naming, though; perhaps revision:insertion/revision:deletion or change:insertion/change:deletion. Note that we may eventually want supplementary attributes such as revision-author and revision-date.

@asurkov, thoughts?

@aleventhal
Copy link
Contributor Author

I'm fine with revisition:insertion|deletion. Can we do that? We'll need to update the docs and file an issue for Firefox, right? I'll take care of asking Windows AT vendors to add support.

Here's the Chrome issue where I'm working on it: https://bugs.chromium.org/p/chromium/issues/detail?id=843744

@asurkov
Copy link
Member

asurkov commented May 22, 2018

Agreed, IA2 text attributes look nicely for text marked by HTML ins/del elements. However these elements have @cite and @datetime attributes, which may be of interest of AT as well.

We have 5 years old bug (https://bugzilla.mozilla.org/show_bug.cgi?id=903187), where I mentioned, that Jamie twitted to go with object attributes, which means, to create accessible objects for elements. Also, new ARIA roles will fit better into ARIA concept, which traditionally requires to create accessible object for elements having @ROLE attribute (expect none/presentation roles for sure).

In short, I like text attributes conceptually, but it looks it is less flexible approach. At least, until we extend text attributes API to expose any value, like we did for object attributes.

@aleventhal
Copy link
Contributor Author

aleventhal commented May 22, 2018

Talked to @asurkov on IRC. We could also use IA2_ROLE_CONTENT_INSERTION and IA2_ROLE_CONTENT_DELETION with the following object attributes "cite" and "datetime". @jcsteh what do you think? Would we still need text attributes or would that be redundant?

@jcsteh
Copy link
Collaborator

jcsteh commented May 22, 2018

@asurkov commented on May 23, 2018, 3:01 AM GMT+10:

However these elements have @cite and @datetime attributes, which may be of interest of AT as well.

Ug. I forgot about these. Still, we could just expose their values as text attributes too if we went with text attributes.

We have 5 years old bug (https://bugzilla.mozilla.org/show_bug.cgi?id=903187), where I mentioned, that Jamie twitted to go with object attributes, which means, to create accessible objects for elements.

Oh. I'm not quite sure why my past self decided that. I guess it was in response to needing to deal with the cite attribute.

In short, I like text attributes conceptually, but it looks it is less flexible approach. At least, until we extend text attributes API to expose any value, like we did for object attributes.

I don't see a big issue with exposing cite and datetime as text attributes: revision-cite, revisoin-datetime. Am I missing something?

I'd like @michaelDCurran's thoughts here. I know we discussed text attributes last time we spoke about this, but we didn't think about the cite attribute, which allows you to specify a URL describing the change. (That said, does anyone even use this?) However, it also occurred to me that we'll probably want to support quick nav to annotations and that might be a bit tricky for text attributes. Note that we'd have that same problem for spelling errors, though.

@joanmarie, I'd also like your perspective on this regarding ATK, given that we generally try to keep IA2 and ATK in sync. In the past, you've expressed a preference for objects over text attributes for sup/sub. Does this preference also apply to ins/del? Does ATK have roles for these already?

@aleventhal
Copy link
Contributor Author

Spelling/grammar are already handled via invalid:spelling|grammar text attribute.
@jcsteh I didn't see you address the idea of using a role. This is what Mac does. We can use object attributes for cite and datetime.

@asurkov
Copy link
Member

asurkov commented May 23, 2018

Seems Mac set a precedence, and thus it's easier to expose accessible objects for ins/del elements for all platforms. @aleventhal suggested to expose both, which may be reasonable, if the consensus is we want to expose those via text attributes on IA2/ATK. I'm not sure whether we need specialized roles for the elements in this case or we can go with generic ones.

@aleventhal
Copy link
Contributor Author

@asurkov , I actually prefer just the new roles and no text attributes. I'll check with Mick. I consider the @cite and @datetime to be red herrings, but they could be exposed as object attributes.

@aleventhal
Copy link
Contributor Author

Mick said he's ok with the roles. I tried to fork so that I could create a pull request, but I'm not permitted. Here is my suggested added text to AccessibleRoles.idl:

/**

  • Content previously deleted or proposed to be deleted, e.g. in revision
  • history or a content view providing suggestions from reviewers.
    */
    IA2_ROLE_CONTENT_DELETION,

/**

  • Content previously inserted or proposed to be inserted, e.g. in revision
  • history or a content view providing suggestions from reviewers.
    */
    IA2_ROLE_CONTENT_INSERTION

@asurkov
Copy link
Member

asurkov commented May 24, 2018 via email

@aleventhal
Copy link
Contributor Author

Mick was fine with the roles in the end, because of consistency with HTML/ARIA/Mac. Can someone add something like this to the IDL? I tried to fork in order to do a pull req but didn't have permissions.

/**

  • Content previously deleted or proposed to be deleted, e.g. in revision
  • history or a content view providing suggestions from reviewers.
    */
    IA2_ROLE_CONTENT_DELETION,

/**

  • Content previously inserted or proposed to be inserted, e.g. in revision
  • history or a content view providing suggestions from reviewers.
    */
    IA2_ROLE_CONTENT_INSERTION

@asurkov
Copy link
Member

asurkov commented May 24, 2018

@aleventhal I sent a request to allow forking and also created a pull request for your change #5

aarongable pushed a commit to chromium/chromium that referenced this issue May 25, 2018
In HTML via <ins>/<del>, and planned for ARIA 1.2, it is possible to mark insertions and
deletions in a document. These need to be exposed to screen readers so that they can
read revision changes or change suggestions from document reviewers.

For now, ins/del are now mapped to internal roles, automation roles and OS X subroles.
We are waiting for roles from IA2 for the new roles there. Here is the github issue:
LinuxA11y/IAccessible2#4

TBR=spang@chromium.org

Bug: 843744
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I90e6d3140f6136a71e0d2f49b0b8d615485536bb
Reviewed-on: https://chromium-review.googlesource.com/1067362
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561920}
@jcsteh
Copy link
Collaborator

jcsteh commented Jul 26, 2018

#5 was merged and adds the roles. Closing.

@jcsteh jcsteh closed this as completed Jul 26, 2018
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

No branches or pull requests

3 participants