|
|
Chromium Code Reviews|
Created:
4 years, 9 months ago by dmazzoni Modified:
4 years, 9 months ago Reviewers:
David Tseng CC:
chromium-reviews, aboxhall+watch_chromium.org, nektar+watch_chromium.org, jam, yuzo+watch_chromium.org, je_julie, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTalkBack needs to know supported HTML element types
TalkBack wants to access the element type navigation we added
in M50 (http://crbug.com/583095), but we need to export the set of
supported element types so it knows which ones it can use.
BUG=592901, 583095
Committed: https://crrev.com/3cbcd14c48a113a4288f1ae70aca7a24b718317d
Cr-Commit-Position: refs/heads/master@{#380255}
Patch Set 1 #Patch Set 2 : Alternate approach #
Messages
Total messages: 21 (7 generated)
Description was changed from ========== Android protocol version BUG= ========== to ========== TalkBack needs version number for element type navigation TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but in order to know whether it's supported or not we need to add some sort of version number to AccessibleNodeInfo and/or AccessibleEvent so they know if they're talking to a recent version of Chrome or not. BUG=592901,583095 ==========
dmazzoni@chromium.org changed reviewers: + dtseng@google.com
dmazzoni@chromium.org changed reviewers: + dtseng@chromium.org - dtseng@google.com
Couldn't they just test if the relevant API is present? Sorry for my ignorance. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Unfortunately in this particular case the API hasn't changed, either way they're calling AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, but they're passing it new strings that weren't previously supported. The previous behavior when passed an unsupported string was just to return the next element of any type, so it's not really possible to detect by trying it out. In general we anticipate making future behavioral changes that warrant TalkBack knowing if it's talking to a recent version of the code or not, so having a version we can increment in the future is good. On Tue, Mar 8, 2016 at 9:25 AM Nektarios Paisios <nektar@google.com> wrote: > Couldn't they just test if the relevant API is present? > Sorry for my ignorance. > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/03/08 17:30:12, dmazzoni wrote: > Unfortunately in this particular case the API hasn't changed, either way > they're calling AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, but they're > passing it new strings that weren't previously supported. The previous > behavior when passed an unsupported string was just to return the next > element of any type, so it's not really possible to detect by trying it out. > Why not just have an attribute called HTMLElementNames and never worry about versioning? > In general we anticipate making future behavioral changes that warrant > TalkBack knowing if it's talking to a recent version of the code or not, so > having a version we can increment in the future is good. > > On Tue, Mar 8, 2016 at 9:25 AM Nektarios Paisios <mailto:nektar@google.com> wrote: > > > Couldn't they just test if the relevant API is present? > > Sorry for my ignorance. > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
Adding Zach, Steven, and Victor to the conversation. +Zach Kuznia <zork@google.com> +Steven Dao <sdao@google.com> +Victor Tsaran <vtsaran@google.com> On Tue, Mar 8, 2016 at 11:44 AM <dtseng@chromium.org> wrote: > Why not just have an attribute called HTMLElementNames and never worry > about > versioning? > Because we'd have to populate it every time, on every element, with a large list of supported actions. A version number allows us to define an ad-hoc protocol somewhere and adding a single attribute advertises that you conform to that protocol. Open to alternative suggestions. We could put the list of supported element types only on the root element or something like that. - Dominic -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On Tue, Mar 8, 2016 at 12:24 PM, Dominic Mazzoni <dmazzoni@chromium.org> wrote: > Adding Zach, Steven, and Victor to the conversation. > +Zach Kuznia <zork@google.com> +Steven Dao <sdao@google.com> +Victor > Tsaran <vtsaran@google.com> > > On Tue, Mar 8, 2016 at 11:44 AM <dtseng@chromium.org> wrote: > >> Why not just have an attribute called HTMLElementNames and never worry >> about >> versioning? >> > > Because we'd have to populate it every time, on every element, with a > large list of supported actions. > Just do it on the document root? > > A version number allows us to define an ad-hoc protocol somewhere and > adding a single attribute advertises that you conform to that protocol. > The issue with a version (as it looks now) is it's not clear what's included and what changed. > > Open to alternative suggestions. We could put the list of supported > element types only on the root element or something like that. > > Yes. > - Dominic > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Setting the element type on the document root would work. TalkBack would have to do a traversal up the tree to the root every time the user navigated by HTML element, but the cost shouldn't be too high (considering that the user would be navigating at max a few elements per second). One concern that I have would be that, if we extend the TalkBack/Chrome contract to include other information, then we'd end up adding a lot of additional stuff on the root node of the WebView. But that's not too terrible since it's just on the single root node. Cheers! Steven On Tue, Mar 8, 2016 at 12:33 PM David Tseng <dtseng@chromium.org> wrote: > On Tue, Mar 8, 2016 at 12:24 PM, Dominic Mazzoni <dmazzoni@chromium.org> > wrote: > >> Adding Zach, Steven, and Victor to the conversation. >> +Zach Kuznia <zork@google.com> +Steven Dao <sdao@google.com> +Victor >> Tsaran <vtsaran@google.com> >> >> On Tue, Mar 8, 2016 at 11:44 AM <dtseng@chromium.org> wrote: >> >>> Why not just have an attribute called HTMLElementNames and never worry >>> about >>> versioning? >>> >> >> Because we'd have to populate it every time, on every element, with a >> large list of supported actions. >> > > Just do it on the document root? > >> >> A version number allows us to define an ad-hoc protocol somewhere and >> adding a single attribute advertises that you conform to that protocol. >> > > The issue with a version (as it looks now) is it's not clear what's > included and what changed. > >> >> Open to alternative suggestions. We could put the list of supported >> element types only on the root element or something like that. >> >> Yes. > >> - Dominic >> >> > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Description was changed from ========== TalkBack needs version number for element type navigation TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but in order to know whether it's supported or not we need to add some sort of version number to AccessibleNodeInfo and/or AccessibleEvent so they know if they're talking to a recent version of Chrome or not. BUG=592901,583095 ========== to ========== TalkBack needs to know supported HTML element types TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but we need to export the set of supported element types so it knows which ones it can use. BUG=592901,583095 ==========
Updated to return a list of supported element types on the root. Key: "ACTION_ARGUMENT_HTML_ELEMENT_STRING_VALUES" Example value: "ARTICLE,BUTTON,CHECKBOX,COMBOBOX,CONTROL,FOCUSABLE,FRAME,GRAPHIC,H1,H2,H3,H4,H5,H6,HEADING,LANDMARK,LINK,LIST,LIST_ITEM,MAIN,MEDIA,RADIO,SECTION,TABLE,TEXT_FIELD,UNVISITED_LINK,VISITED_LINK"
Hi Dominic. What do we do about https://bugs.chromium.org/p/chromium/issues/detail?id=591523? Should the title be changed or new bug opened? I am filing a reference feature request for ourselves and was wondering what I should be referring to now! :) On Tue, Mar 8, 2016 at 12:55 PM, Steven Dao <sdao@google.com> wrote: > Setting the element type on the document root would work. TalkBack would > have to do a traversal up the tree to the root every time the user > navigated by HTML element, but the cost shouldn't be too high (considering > that the user would be navigating at max a few elements per second). > > One concern that I have would be that, if we extend the TalkBack/Chrome > contract to include other information, then we'd end up adding a lot of > additional stuff on the root node of the WebView. But that's not too > terrible since it's just on the single root node. > > Cheers! > Steven > > On Tue, Mar 8, 2016 at 12:33 PM David Tseng <dtseng@chromium.org> wrote: > >> On Tue, Mar 8, 2016 at 12:24 PM, Dominic Mazzoni <dmazzoni@chromium.org> >> wrote: >> >>> Adding Zach, Steven, and Victor to the conversation. >>> +Zach Kuznia <zork@google.com> +Steven Dao <sdao@google.com> +Victor >>> Tsaran <vtsaran@google.com> >>> >>> On Tue, Mar 8, 2016 at 11:44 AM <dtseng@chromium.org> wrote: >>> >>>> Why not just have an attribute called HTMLElementNames and never worry >>>> about >>>> versioning? >>>> >>> >>> Because we'd have to populate it every time, on every element, with a >>> large list of supported actions. >>> >> >> Just do it on the document root? >> >>> >>> A version number allows us to define an ad-hoc protocol somewhere and >>> adding a single attribute advertises that you conform to that protocol. >>> >> >> The issue with a version (as it looks now) is it's not clear what's >> included and what changed. >> >>> >>> Open to alternative suggestions. We could put the list of supported >>> element types only on the root element or something like that. >>> >>> Yes. >> >>> - Dominic >>> >>> >> -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I think we should use the same bug, we're just solving the issue a different way On Tue, Mar 8, 2016 at 2:29 PM 'Victor Tsaran' via Chromium-reviews < chromium-reviews@chromium.org> wrote: > Hi Dominic. > What do we do about > https://bugs.chromium.org/p/chromium/issues/detail?id=591523? Should the > title be changed or new bug opened? > I am filing a reference feature request for ourselves and was wondering > what I should be referring to now! :) > > > On Tue, Mar 8, 2016 at 12:55 PM, Steven Dao <sdao@google.com> wrote: > >> Setting the element type on the document root would work. TalkBack would >> have to do a traversal up the tree to the root every time the user >> navigated by HTML element, but the cost shouldn't be too high (considering >> that the user would be navigating at max a few elements per second). >> >> One concern that I have would be that, if we extend the TalkBack/Chrome >> contract to include other information, then we'd end up adding a lot of >> additional stuff on the root node of the WebView. But that's not too >> terrible since it's just on the single root node. >> >> Cheers! >> Steven >> >> On Tue, Mar 8, 2016 at 12:33 PM David Tseng <dtseng@chromium.org> wrote: >> >>> On Tue, Mar 8, 2016 at 12:24 PM, Dominic Mazzoni <dmazzoni@chromium.org> >>> wrote: >>> >>>> Adding Zach, Steven, and Victor to the conversation. >>>> +Zach Kuznia <zork@google.com> +Steven Dao <sdao@google.com> +Victor >>>> Tsaran <vtsaran@google.com> >>>> >>>> On Tue, Mar 8, 2016 at 11:44 AM <dtseng@chromium.org> wrote: >>>> >>>>> Why not just have an attribute called HTMLElementNames and never worry >>>>> about >>>>> versioning? >>>>> >>>> >>>> Because we'd have to populate it every time, on every element, with a >>>> large list of supported actions. >>>> >>> >>> Just do it on the document root? >>> >>>> >>>> A version number allows us to define an ad-hoc protocol somewhere and >>>> adding a single attribute advertises that you conform to that protocol. >>>> >>> >>> The issue with a version (as it looks now) is it's not clear what's >>> included and what changed. >>> >>>> >>>> Open to alternative suggestions. We could put the list of supported >>>> element types only on the root element or something like that. >>>> >>>> Yes. >>> >>>> - Dominic >>>> >>>> >>> > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to chromium-reviews+unsubscribe@chromium.org. > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
lgtm Thanks for changing to this approach.
The CQ bit was checked by dmazzoni@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1767313002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1767313002/20001
Message was sent while issue was closed.
Description was changed from ========== TalkBack needs to know supported HTML element types TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but we need to export the set of supported element types so it knows which ones it can use. BUG=592901,583095 ========== to ========== TalkBack needs to know supported HTML element types TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but we need to export the set of supported element types so it knows which ones it can use. BUG=592901,583095 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== TalkBack needs to know supported HTML element types TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but we need to export the set of supported element types so it knows which ones it can use. BUG=592901,583095 ========== to ========== TalkBack needs to know supported HTML element types TalkBack wants to access the element type navigation we added in M50 (http://crbug.com/583095), but we need to export the set of supported element types so it knows which ones it can use. BUG=592901,583095 Committed: https://crrev.com/3cbcd14c48a113a4288f1ae70aca7a24b718317d Cr-Commit-Position: refs/heads/master@{#380255} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/3cbcd14c48a113a4288f1ae70aca7a24b718317d Cr-Commit-Position: refs/heads/master@{#380255} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
