Closed
Bug 1451241
Opened 6 years ago
Closed 6 years ago
Clicking "Inspect Accessibility" option from the context menu doesn't highlight selected items in properties panel
Categories
(DevTools :: Accessibility Tools, defect)
Tracking
(firefox61 verified)
VERIFIED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | verified |
People
(Reporter: laszlo.bialis, Assigned: yzen)
References
Details
Attachments
(1 file, 1 obsolete file)
10.13 KB,
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
[Version]: 61.0a1 [Build ID]: 20180327120140 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:61.0) Gecko/20100101 Firefox/61.0 [Prerequisites]: * Have the latest try build 61.0a1 from (2018-03-27) installed. * Accessibility is enabled and accessibility tab is opened [Steps]: 1. Right click on an element from the page's content. 2. Select and click the "Inspect Accessibility" option from the context menu. [Expected result]: 1.The element inspected with Accessibility can be highlighted with a purple transparent overlay for ~2 seconds. 2.The line related to the Accessibility inspect is selected and highlighted. 3.The attributes that are updated from the "Properties" panel are selected and highlighted with an orange background that lasts ~2 seconds. [Actual result]: Expected result 1 is ok, but the 2nd and 3rd option is not observable, as such the line related to the Accessibility inspect is NOT selected and NOT highlighted and the attributes that are updated from the "Properties" panel are NOT selected and NOT highlighted with an orange background that lasts ~2 seconds.
Reporter | ||
Comment 1•6 years ago
|
||
The issue reproduces on [Version]: 61.0a1 [Build ID]: 20180411100123 Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0 also using High Contrast mode on Win 10
Assignee | ||
Comment 2•6 years ago
|
||
(In reply to laszlo.bialis from comment #0) > [Version]: 61.0a1 > [Build ID]: 20180327120140 > Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:61.0) Gecko/20100101 > Firefox/61.0 > > [Prerequisites]: > * Have the latest try build 61.0a1 from (2018-03-27) installed. > * Accessibility is enabled and accessibility tab is opened > > [Steps]: > 1. Right click on an element from the page's content. > 2. Select and click the "Inspect Accessibility" option from the context menu. > > [Expected result]: > 1.The element inspected with Accessibility can be highlighted with a purple > transparent overlay for ~2 seconds. > 2.The line related to the Accessibility inspect is selected and highlighted. > 3.The attributes that are updated from the "Properties" panel are selected > and highlighted with an orange background that lasts ~2 seconds. > > [Actual result]: > Expected result 1 is ok, but the 2nd and 3rd option is not observable, as > such the line related to the Accessibility inspect is NOT selected and NOT > highlighted and the attributes that are updated from the "Properties" panel > are NOT selected and NOT highlighted with an orange background that lasts ~2 > seconds. Organge highlight is no longer shown on underlying accessible object changes in properties sidebar. They only flash when property for the same accessible object changes dynamically.
Assignee | ||
Comment 3•6 years ago
|
||
Hi Laszlo, could you see if this build (with potential fix) has this problem fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e1594cd534c2d87a3d8f2912530432d18750e50 ?
Flags: needinfo?(laszlo.bialis)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•6 years ago
|
||
Hey! I've tried with your build (Version: 61.0a1, Build ID:20180413130246 )using the above steps and found the following: a.) At the very first try (first opening of the build and first activation of the accessibility) the issue is partially reproducible, with the following results: 1. The element inspected with Accessibility can be highlighted with a purple transparent overlay for ~2 seconds. --> Actually the item remains highlighted until another selection is made. 2. The line related to the Accessibility inspect is selected and highlighted. --> Works ok 3. The attributes that are updated from the "Properties" panel are selected and highlighted with an orange background that lasts ~2 seconds. --> no highlight is done whatsoever in the properties panel b.) If one uses the "Pick accessible object from the page" option and afterwards uses the right click and click on the "Inspect Accessibility" option from the context menu, the following happens: 1. The element inspected with Accessibility can be highlighted with a purple transparent overlay for ~2 seconds. --> Works ok 2. The line related to the Accessibility inspect is selected and highlighted. --> Works ok 3. The attributes that are updated from the "Properties" panel are selected and highlighted with an orange background that lasts ~2 seconds. --> There is no orange background highlighted in the Properties panel, but a highlight is done on the name of the item selected as in the previous step.
Flags: needinfo?(laszlo.bialis)
Assignee | ||
Comment 5•6 years ago
|
||
Couple of things to note: (In reply to laszlo.bialis from comment #4) > Hey! > > I've tried with your build (Version: 61.0a1, Build ID:20180413130246 )using > the above steps and found the following: > > a.) At the very first try (first opening of the build and first activation > of the accessibility) the issue is partially reproducible, with the > following results: > 1. The element inspected with Accessibility can be highlighted with a > purple transparent overlay for ~2 seconds. --> Actually the item remains > highlighted until another selection is made. Are you sure that the mouse was not hovering over an element in the Accessibility tree? > 2. The line related to the Accessibility inspect is selected and > highlighted. --> Works ok > 3. The attributes that are updated from the "Properties" panel are > selected and highlighted with an orange background that lasts ~2 seconds. > --> no highlight is done whatsoever in the properties panel This is expected, not orange highlight should happen on new selection. > > b.) If one uses the "Pick accessible object from the page" option and > afterwards uses the right click and click on the "Inspect Accessibility" > option from the context menu, the following happens: > 1. The element inspected with Accessibility can be highlighted with a > purple transparent overlay for ~2 seconds. --> Works ok > 2. The line related to the Accessibility inspect is selected and > highlighted. --> Works ok > 3. The attributes that are updated from the "Properties" panel are > selected and highlighted with an orange background that lasts ~2 seconds. > --> There is no orange background highlighted in the Properties panel, but a > highlight is done on the name of the item selected as in the previous step. This is expected, not orange highlight should happen on new selection.
Assignee | ||
Comment 6•6 years ago
|
||
Attachment #8969720 -
Flags: review?(jdescottes)
Comment 7•6 years ago
|
||
Comment on attachment 8969720 [details] [diff] [review] 1451241 patch Review of attachment 8969720 [details] [diff] [review]: ----------------------------------------------------------------- If I understand correctly, here we are trying to infer that a user clicked on a text node. This information is missing from the context menu code, which simply gives us a selector to an element. This is tough to fix without changing the context menu code to give us some extra information here. My comments below are just to make sure we are have the same understanding of the limitations of the current patch. "inlineTextChild" is a very special case of nodes that have a single text child, which is smaller than a given length. If we modify slightly the test page of your mochitest to replace "text" by a bigger string: data:text/html;charset=UTF-8,%3Ch1 id%3D"h1"%3Eheader%3C%2Fh1%3E%3Cp id%3D"p"%3Eparagraph%3C%2Fp%3E%3Cspan id%3D"span"%3EIamaverylongtextwhichdoesntfitinInlineTextChildReallyIdontIamtoobig%3C%2Fspan%3E right clicking on the long text and selecting "Inspect Accessibility Properties" doesn't work. Instead we could use the walker to get first textNode from the children of the current node. Now let's consider the following markup: data:text/html,<span><span>[-a-]</span>[do-not-inspect-me]<span>[-b-]</span>[inspect-me]</span> Both "[do-not-inspect-me]" and "[inspect-me]" are text nodes, and if we right click on "[inspect-me]" I guess you expect to see it selected in the accessibility panel. But this case really can't be solved without getting more information about the clicked node. I am clearing the review flag for now, I think we should really not use inlineTextChild but rather try to get a child text node. And we should have a comment that mentions that this is a best-effort approach etc... Hope this makes sense!
Attachment #8969720 -
Flags: review?(jdescottes)
Assignee | ||
Comment 8•6 years ago
|
||
Just a couple of questions/concerns: - is inlineTextChild the text child that is wrapped inside a node (e.g. in the inspector tree it shows up as inner text of the node), instead of allowing for the parent node to be expanded? If so then I think this is exactly what I need at least for the inspection from the inspector panel. Some context: when the parent node can be expanded for a text node, then it is fine to not have the accessibility look up since it indeed does not have an accessible object. Its text child (that can be seen when the node can be expanded) will have an accessible, so trying to inspect a11y object for it will work. However when the since text child is embedded into the parent node, we want to do that lookup because otherwise there is no way to inspect a11y object for the text node. If this is what inlineTextChild is for then it's exactly what I need. - inspecting from the content page is harder. I see that selectors for inner text child will have a "...:nth-child(2)" or similar selectors, but querySelector will just return the parent. I can probably stop on the first text child that I find has an accessible, but then it's not ideal with the second example that you provided. There if I click on the second text child gets more complicated. Should I consider parsing such selectors to grab the nth child of the node that was found or should I just get the first text child for this iteration? Thanks
Flags: needinfo?(jdescottes)
Comment 9•6 years ago
|
||
(In reply to Yura Zenevich [:yzen] from comment #8) > Just a couple of questions/concerns: > > - is inlineTextChild the text child that is wrapped inside a node (e.g. in > the inspector tree it shows up as inner text of the node), instead of > allowing for the parent node to be expanded? If so then I think this is > exactly what I need at least for the inspection from the inspector panel. > Some context: when the parent node can be expanded for a text node, then it > is fine to not have the accessibility look up since it indeed does not have > an accessible object. Its text child (that can be seen when the node can be > expanded) will have an accessible, so trying to inspect a11y object for it > will work. However when the since text child is embedded into the parent > node, we want to do that lookup because otherwise there is no way to inspect > a11y object for the text node. If this is what inlineTextChild is for then > it's exactly what I need. > Yes inlineTextChild is exactly that, so for a scenario where we go from inspector to a11y panel it should be enough. > - inspecting from the content page is harder. I see that selectors for inner > text child will have a "...:nth-child(2)" or similar selectors, but > querySelector will just return the parent. I can probably stop on the first > text child that I find has an accessible, but then it's not ideal with the > second example that you provided. There if I click on the second text child > gets more complicated. Should I consider parsing such selectors to grab the > nth child of the node that was found or should I just get the first text > child for this iteration? > I focused on this scenario since this is what I could understand from the bug and the test :) I think stopping on the first text child is fine for now and we can improve this in a follow up.
Flags: needinfo?(jdescottes)
Assignee | ||
Comment 10•6 years ago
|
||
List of updates: - added test for non inline text child - enabled inspect a11y menu item for text nodes - select first accessible text node child for an element that does not have accessible.
Attachment #8969720 -
Attachment is obsolete: true
Attachment #8970570 -
Flags: review?(jdescottes)
Comment 11•6 years ago
|
||
Comment on attachment 8970570 [details] [diff] [review] 1451241 patch v2 Review of attachment 8970570 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me thanks!
Attachment #8970570 -
Flags: review?(jdescottes) → review+
Comment 12•6 years ago
|
||
Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/71105dc996c9 check text children of non-accessible nodes when retrieving accessible objects. r=jdescottes
Comment 13•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/71105dc996c9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment 14•6 years ago
|
||
On the latest Firefox Nightly (Build ID: 20180426220144) the issue cannot be reproduced by following the steps and expected results provided in Comment 5 on Mac OS X 10.12, Windows 10 x64 and Ubuntu 16.04 x64, thus I will close the issue as VERIFIED FIXED.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•