Checkbox label for Shockwave Flash in about:addons plugins has improper tab_focus

RESOLVED FIXED in Firefox 66

Status

()

defect
--
minor
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: cfogel, Assigned: toms, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla66
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Details

(Whiteboard: [lang=css])

Attachments

(3 attachments)

(Reporter)

Description

5 months ago
[Affected versions]:
- 45.3.0esr, 63.0b3, 64.0b10, 65.0a1 (2018-11-20)  

[Affected platforms]:
- win10x64, Ubuntu18.04;

[Steps to reproduce]:
1. Launch Firefox;
2. Access about:addons
3. Click on the Plugins section;
4. (Double)Click on the Showckwave Flash item;
5. Drag and select the Learn More.. text (so it has the dotted outline);
6. Press the Tab key on the keyboard;

[Expected result]:
- the checkbox is marked and selected;

[Actual result]:
- the dotted select line is on the dummy label and not the checkbox;

[Regression range]:
*mozregression run where the considered good version did not apear to have the checkbox at all:
- last good: 2017-04-13 changeset: 819a666afddc804b6099ee1b3cff3a0fdf35ec15
- first bad: 2017-04-14 chngeset: cda24082bff8864a6e53726feeae33cae9e17309
pushlog url: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c40ca7a1bdd93632c6bdc5e23bd33d984d508b19&tochange=308cdb913d717f8edc932f5bac93854e3e54b84d

*mozregression run where the considered good version still had the "line" smaller ~2-3pixels;
- last good: 2017-11-14 changeset: f0c0fb9182d695081edf170d8e3bcb8164f2c96a
- first bad: 2017-11-15 chngeset: e070277ec199fa96fa490ed52d33646a376d0d80

[Additional notes]:
- attached screenshot with the issue;
- on macOS the tabIndex skips this checkbox;
- for Ubuntu Shockwave Flash needs to be added;
Felipe, can you look in this?
Blocks: 1348089
Flags: needinfo?(felipc)
Discussed during triage, won't fixing this for 63 and ESR.
So I think the main problem is that this is a checkbox with no text content for the label. A similar problem was handled in bug 669507. Although that bug title appears to have solved the problem for all cases, in reality that bug only fixed it with a workaround.

I think we should just do the same workaround here and not display the inner .checkbox-label-box for this #pluginFlashBlockingCheckbox checkbox.
Flags: needinfo?(felipc)
Keywords: regression
(Reporter)

Updated

5 months ago
QA Whiteboard: regression
(Assignee)

Comment 5

5 months ago
Hello. I'm happy to take this as my first contribution.
(In reply to Jared Wein [:jaws] (Regression Engineering Owner for 65) (please needinfo? me) from comment #1)
> Felipe, can you look in this?

hey! can i take this bug?
Hi Tom, are you interested in taking this bug?

Do you already have a copy of the source code, and have you set up the things needed to compile Firefox? If not yet, I recommend that you take a look at this page: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds
Assignee: nobody → causality1
Status: NEW → ASSIGNED
Flags: needinfo?(causality1)
(In reply to Abhishek Chandrasenan [:acnair] from comment #6)
> 
> hey! can i take this bug?

Hi Abhishek, Tom had asked first to work on this bug, so let's give him a chance to work on it. Let me know if you need any help on finding some other good-first-bug to take
(Assignee)

Comment 9

5 months ago
(In reply to :Felipe Gomes (needinfo me!) from comment #7)
> Hi Tom, are you interested in taking this bug?
> 
> Do you already have a copy of the source code, and have you set up the
> things needed to compile Firefox? If not yet, I recommend that you take a
> look at this page:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Artifact_builds

Hey Felipe. Yes, I've compiled the 65.0a1 nightly and have arc and moz phab installed. I should be good to go.
Flags: needinfo?(causality1)
(In reply to :Felipe Gomes (needinfo me!) from comment #8)
> (In reply to Abhishek Chandrasenan [:acnair] from comment #6)
> > 
> > hey! can i take this bug?
> 
> Hi Abhishek, Tom had asked first to work on this bug, so let's give him a
> chance to work on it. Let me know if you need any help on finding some other
> good-first-bug to take

hey that's cool! I will contact you in IRC! :D thanks for reaching out
(Assignee)

Comment 11

5 months ago
Adding that line doesn't seem to stop it from focusing on the label. Unless I'm doing something wrong here

https://imgur.com/vbjZUT4
Flags: needinfo?(felipc)
(Assignee)

Comment 12

4 months ago
Hid the checkbox-label-box from the Shockwave plugin to correct the tab focus.

Comment 13

4 months ago
Pushed by fgomes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad217069266e
Checkbox label for Shockwave Flash in about:addons plugins has improper tab_focus r=Felipe
Flags: needinfo?(felipc)

Comment 14

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ad217069266e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Thanks so  much, Tom! Your contribution has been added to our recognition wiki (https://wiki.mozilla.org/Add-ons/Contribute/Recognition) and your Mozillians profile has been vouched. :) 

Welcome onboard! We look forward to seeing you around the project.
Did you want to nominate this for Beta uplift?
Flags: qe-verify+
Flags: needinfo?(felipc)
Nope, not necessary as it's not really a regression, it has existed one way or another since we added that checkbox
Flags: needinfo?(felipc)
(Assignee)

Comment 18

4 months ago
(In reply to Caitlin Neiman [:caitmuenster] from comment #15)
> Thanks so  much, Tom! Your contribution has been added to our recognition
> wiki (https://wiki.mozilla.org/Add-ons/Contribute/Recognition) and your
> Mozillians profile has been vouched. :) 
> 
> Welcome onboard! We look forward to seeing you around the project.

Thanks a lot Caitlin!

Comment 19

4 months ago
I checked the issue on Nightly 66.0a1 (2019-01-03). 

- On Windows 10, the checkbox can be marked with tab->space, but it is not highlighted.
- On Ubuntu 16.04, the checkbox can be marked with tab->space, it is not highlighted and the dotted line appears right next to the checkbox as seen in the printscreen attached.

Should I create a new bug and mark this as verified or reopen this one?
Flags: needinfo?(causality1)

Comment 20

4 months ago
Posted image checkbox.png
(Assignee)

Comment 21

3 months ago

I'm using the same Ubuntu version but the line is not present. I don't know if this should be reopened or not, maybe @Felipe can let us know.

Flags: needinfo?(causality1)

(In reply to David Olah from comment #19)

I checked the issue on Nightly 66.0a1 (2019-01-03).

  • On Windows 10, the checkbox can be marked with tab->space, but it is not
    highlighted.
  • On Ubuntu 16.04, the checkbox can be marked with tab->space, it is not
    highlighted and the dotted line appears right next to the checkbox as seen
    in the printscreen attached.

Should I create a new bug and mark this as verified or reopen this one?

The screenshot you provided is not from Nightly, but it's from a release version of Firefox. Are you sure you checked the right version?

This bug was fixed during the 66 cycle, so it's only gonna make it to Beta at the end of January and to Release in the beginning of March.

Flags: needinfo?(david.olah)

Comment 23

3 months ago

Indeed the screenshot is made on release. The dotted line appears only there and not on Nightly.

The biggest concern and question is that, if the checkbox is not highlighted when it is selected with tab, even if it can be checked with space at the moment, the issue should be closed or not?

Flags: needinfo?(david.olah)
You need to log in before you can comment on or make changes to this bug.