Closed Bug 1591848 Opened 5 years ago Closed 1 year ago

Permanent search icon in front of megabar has no tooltip/accessibility label and no keyboard functionality

Categories

(Firefox :: Address Bar, defect, P3)

defect
Points:
3

Tracking

()

RESOLVED WORKSFORME
Accessibility Severity s2

People

(Reporter: Jamie, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: access, blocked-ux)

Just for something new and different 😒, we have a new toolbar button - the permanent search icon introduced in bug 1589836 - which has no tooltip/accessibility label and cannot be reached nor activated via the keyboard (space/enter when focused). I realise this is unfinished, but I'm filing this now because otherwise it's likely to be missed.

  1. Put role="button" on #urlbar-search-icon. As well as exposing the correct accessibility role, that will make it reachable with toolbar keyboard navigation.
  2. Support activation with space/enter. I don't know how this thing handles mouse clicks, but it doesn't seem to respond to click (which the toolbar key nav code fires by default), so it'll need to support keypress itself.
  3. Give it a tooltip with tooltiptext or an accessibility label with aria-label. A tooltip is more ideal because it makes the purpose clear to users who might not use screen readers but might (for whatever reason) have trouble determining the purpose of the icon. Unless the icon already contains text?

Bug 1590374 is tracking the tooltip. The rest depends on the finalized design we are waiting for.

Depends on: 1590374
Priority: -- → P3

(In reply to James Teh [:Jamie] from comment #0)

Just for something new and different 😒, we have a new toolbar button - the permanent search icon introduced in bug 1589836 - which has no tooltip/accessibility label and cannot be reached nor activated via the keyboard (space/enter when focused).

Clicking the icon currently does the same as Accel+K. Verdi is also considering letting the icon do nothing except for focusing the address bar. Either way it seems redundant to expose it as a button to accessibility software. But as mak said, nothing is set in stone yet as we're waiting for the final UX specification.

Blocks: 1589836
Keywords: regressionblocked-ux
No longer regressed by: 1589836

(In reply to Dão Gottwald [::dao] from comment #2)

(In reply to James Teh [:Jamie] from comment #0)
Clicking the icon currently does the same as Accel+K. Verdi is also considering letting the icon do nothing except for focusing the address bar. Either way it seems redundant to expose it as a button to accessibility software.

I understand the thinking here. It can be argued, though, that the button is no more redundant for keyboard users than it is visually. That is, as I understand it, this is about discoverability. Sure, you can press control+k, but maybe you don't know about that shortcut? In contrast, once you learn toolbar keyboard navigation (and yes, I know that isn't discoverable in itself, but it's one paradigm to learn instead of many), now this function is just as discoverable to a keyboard/screen reader user as it is to anyone else.

As you say, though, let's wait to see what the final UX spec is.

Points: --- → 3
Blocks: 1603165
No longer blocks: urlbar-update-1

Updating the Accessibility Team's impact assessment to conform with the new triage guidelines. See https://wiki.mozilla.org/Accessibility/Triage for descriptions of these whiteboard flags.

Whiteboard: [access-p1] → [access-s2]
Severity: normal → S3

The severity field for this bug is set to S3. However, the accessibility severity is higher, [access-s2].
:adw, could you consider increasing the severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(adw)

Looking at the a11y triage guidelines and comments above, access-s2 is the correct severity for this feature (if not access-s1): "Feature completely unavailable/inaccessible."

It makes sense for the bug severity to match that IMO, so I'll bump it up.

But this feature has always been preffed off and we don't have any plans to ever pref it on, so it should keep a low priority. We should actually just remove it altogether IMO since Verdi was driving this exploration and since he left the team there are no plans to carry it forward AFAIK.

Severity: S3 → S2
Flags: needinfo?(adw)

(In reply to Drew Willcoxon :adw from comment #6)

Looking at the a11y triage guidelines and comments above, access-s2 is the correct severity for this feature (if not access-s1): "Feature completely unavailable/inaccessible."

It makes sense for the bug severity to match that IMO, so I'll bump it up.

But this feature has always been preffed off and we don't have any plans to ever pref it on, so it should keep a low priority. We should actually just remove it altogether IMO since Verdi was driving this exploration and since he left the team there are no plans to carry it forward AFAIK.

If this is preffed off, I don't think S2 is accurate, because this affects 0 users. But also, if we're not carrying this forward, can we remove the feature and "fix" the bug that way? (feel free to forward that q to the product owner of this stuff)

Flags: needinfo?(adw)

Well yes, zero users are currently affected because it's preffed off, but the feature as it exists is broken for users with a11y needs, which qualifies it as access-s2. But if this is on your dashboard or something and you'd like to get rid of it, feel free to remove the access tag (I didn't add it) and/or change the severity.

As I mentioned I think we should just remove this button altogether. It's not my call but I can raise the topic.

Flags: needinfo?(adw)

(In reply to Drew Willcoxon :adw from comment #8)

Well yes, zero users are currently affected because it's preffed off, but the feature as it exists is broken for users with a11y needs, which qualifies it as access-s2.

I'm confused. The feature doesn't exist for users in any meaningful way if it's turned off, I think.

But if this is on your dashboard or something and you'd like to get rid of it, feel free to remove the access tag (I didn't add it) and/or change the severity.

We do track S2 bugs, yes, but I'm not particularly keen to override the decision of the a11y team or a separate frontend team. I'm saying, if we think this is really S2 then it needs resourcing. If it isn't S2, then let's not mark it S2. I'm inclined to think that either the latter is the case, and/or we should just remove all this code if it's been there but preffed off for 2 years already.

As I mentioned I think we should just remove this button altogether. It's not my call but I can raise the topic.

Yes please! Can you forward the needinfo to whoever you raise this to? I'd like to make sure we get a resolution here.

Flags: needinfo?(adw)

(In reply to :Gijs (he/him) from comment #9)

I'm confused. The feature doesn't exist for users in any meaningful way if it's turned off, I think.

If we decided tomorrow to re-enable this feature, then this bug would be a problem we'd need to fix. The feature still exists even if it's preffed off. I don't know what to tell you.

We do track S2 bugs, yes, but I'm not particularly keen to override the decision of the a11y team or a separate frontend team.

I didn't set this as access-s2, the a11y team did. And then the release mgmt bot came along and prompted me to set the bug's severity so it matches the access tag, which is reasonable. A high severity with a low priority seems completely reasonable for this bug.

Anyway, set the priority and severity however you'd like. You can decide.

Yes please! Can you forward the needinfo to whoever you raise this to? I'd like to make sure we get a resolution here.

Nobody in Product even checks Bugzilla. I'm not sure they even have accounts. I'll raise it on Slack or something.

Flags: needinfo?(adw)

(In reply to Drew Willcoxon :adw from comment #10)

(In reply to :Gijs (he/him) from comment #9)

I'm confused. The feature doesn't exist for users in any meaningful way if it's turned off, I think.

If we decided tomorrow to re-enable this feature, then this bug would be a problem we'd need to fix. The feature still exists even if it's preffed off. I don't know what to tell you.

Could we decrement the severity now, and increment it if we decide to ship in future? This is also what happens with e.g. crash bugs which have no noticeable volume right now, if the crash volume changes. It isn't useful to track all bugs that would be bad if they suddenly started affecting tons of users, as if they are already bad - it makes it impossible to prioritize the right things because they get drowned out in amongst all the bugs that aren't actually bad right now, but would be bad if...

We do track S2 bugs, yes, but I'm not particularly keen to override the decision of the a11y team or a separate frontend team.

I didn't set this as access-s2, the a11y team did. And then the release mgmt bot came along and prompted me to set the bug's severity so it matches the access tag, which is reasonable. A high severity with a low priority seems completely reasonable for this bug.

Talking to the a11y folks, I think the intention is to make people think about the relative severities ("could you consider"), but it isn't a given that every bug's conflicting severities should be resolved by incrementing the general severity.

The bug was set to access-s2 when there was a reasonable expectation the new behaviour would be shipping soon ("I realise this is unfinished, but I'm filing this now..."). That was 2.5 years ago. I think instead at this point we should reconsider both severity and access-severity (or, if we're not going to ship this ever, close as wontfix/invalid/worksforme).

Yes please! Can you forward the needinfo to whoever you raise this to? I'd like to make sure we get a resolution here.

Nobody in Product even checks Bugzilla. I'm not sure they even have accounts. I'll raise it on Slack or something.

Thank you. Let us know what you get back?

To offer some more context here: we track and get asked about S1/S2 volume on a weekly basis by the org leadership, because we do not want to indefinitely ship bad bugs to users. For that to work, we need S1/S2 to actually reflect things that are bad, so that when we drive down that list, it isn't wasted effort. Marking high-severity bugs as low priority doesn't help with this (ie the bugs still show up as needing urgent fixing), and in fact is kind of confusing: if the bug is genuinely high severity in terms of how many users it affects and the effect it has for those users, how isn't it a priority?

Flags: needinfo?(adw)

Given this isn't enabled and isn't likely to be enabled any time soon, I think s3 (or even s4) is probably reasonable. I guess it'd make sense for this to be access-s4 as well, since it also isn't a real a11y concern unless it's going to be enabled. On the other hand, this is tricky from the a11y perspective because if we ever did decide to ship this, this would then become an access-s2 again and I worry it would just get lost in the sea of other things, potentially without the a11y team ever knowing about it and re-triaging until it's too late. (That kind of thing happens far too often unfortunately.)

(In reply to :Gijs (he/him) from comment #11)

To offer some more context here: we track and get asked about S1/S2 volume on a weekly basis by the org leadership, because we do not want to indefinitely ship bad bugs to users. For that to work, we need S1/S2 to actually reflect things that are bad, so that when we drive down that list, it isn't wasted effort.

That's fair, though it does suggest we potentially need to look at adjusting our general Firefox severity rating guidance. Right now, s2 says "(Serious) Major Functionality/product severely impaired and a satisfactory workaround doesn’t exist". The functionality in this case is severely impaired and there's no workaround... except that the functionality is disabled for users, so that's academic. But that guidance does not make it at all clear how to handle things that are in development vs shipping to users vs temporarily disabled for users vs likely to be disabled for users forever. We tried to make the access-s ratings closely align to the general severity ratings, but it gets messy in cases like this.

Marking high-severity bugs as low priority doesn't help with this (ie the bugs still show up as needing urgent fixing), and in fact is kind of confusing: if the bug is genuinely high severity in terms of how many users it affects and the effect it has for those users, how isn't it a priority?

I mean, this does raise the question of why we differentiate severity and priority at all, but that's a whole other can of worms.

Severity: S2 → S3
Flags: needinfo?(adw)
Flags: needinfo?(jepstein)

:jamie

The front-end team is looking at this sometime since it's been filed and discussed, and we're not sure what to do here.
The search icon isn't something that has any functionality here presently for any users, so it's not button.
And there is supporting text in the megabar already.

Flags: needinfo?(jepstein) → needinfo?(jteh)

If it doesn't have any functionality for any user, should it just be removed? Then there wouldn't be an a11y bug to fix.

On the other hand, if we're looking at doing something with this at some point, then we need to fix this a11y issue.

Am I missing something here?

Flags: needinfo?(jteh)

(In reply to James Teh [:Jamie] from comment #14)

If it doesn't have any functionality for any user, should it just be removed? Then there wouldn't be an a11y bug to fix.

I think there is significant confusion here about which button / UI element we're even talking about. comment 0 mentions #urlbar-search-icon - but urlbar-search-icon as a string does not occur in current m-c. In comment 8 and later, the discussion seemed to suggest this was a feature that was turned off by default - perhaps it's since been removed entirely, but I'm not sure.

There is a magnifying glass ("search") icon that shows up at the start of the URL bar when the bar is empty or you're typing in it (as opposed to when the URL bar shows the current page's URL, when the icon is replaced with a lock icon or similar). All of this is implemented in #identity-icon-box, which has click and keypress handlers, and role=button. When it shows the search icon, you can't activate it with the mouse - it appears the clicks are swallowed by the URL bar's mousedown handler, which will show/hide the URL bar popup. You also can't use tab + arrows to keyboard focus it in this case. So in this case it behaves effectively as a background image.

Comment 0 suggests this UI element is a button that does something when clicked, but it's not exposed as a button to a11y. That'd obviously be wrong. But because there's no urlbar-search-icon, and because the identity-icon-box element does have role=button, we're confused about what needs to be fixed here.

Your comment suggests you're still seeing this issue on current Firefox and/or Nightly. Can you elaborate on exactly what UI element has an issue for you today?

(My suspicion is that, if anything, the role=button bit should be added/removed from the identity-icon-box element based on whether it is actually doing something in the present, but I (a) don't know how useful that is and (b) how compatible that is with a11y tools, and (c) that's almost the opposite of what this bug got filed for so seems like a weird thing to do to "resolve" this bug. :-) )

Flags: needinfo?(jteh)

There is indeed a disabled feature putting a search button in front of the urlbar, that is not intended to be shipped, nor tested, at this time.
We're not removing it because there's some discussion about a replacement and it's useful as a demo.

This bug was filed assuming that the icon in question was clickable by a mouse user as per comment 3. Comment 15 suggests it isn't, in which case that would make this bug invalid. On the other hand, comment 16 suggests that there is discussion about a replacement, in which case this bug needs to be fixed before said replacement ships.

Flags: needinfo?(jteh)

I talked to Marco about this - according to him, this is turned off and it won't be getting turned on, so tracking this separately from completely removing the code doesn't make sense. I'm going to resolve as WFM given that nobody should be experiencing the behaviour described in comment 0.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WORKSFORME
Accessibility Severity: --- → s2
Whiteboard: [access-s2]
You need to log in before you can comment on or make changes to this bug.