Closed Bug 1371951 Opened 7 years ago Closed 6 years ago

browser_style Checkbox is invisible when not in front of its associated label element

Categories

(WebExtensions :: Frontend, defect, P3)

Product:
WebExtensions
Component:
Frontend
Type:
defect

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: e7358d9c, Assigned: e7358d9c)

References

Details

Attachments

(4 files, 5 obsolete files)

`browser_style` Checkbox Test Extension v2.xpi
(deleted), application/x-xpinstall
Details
format-with-patch.png
(deleted), image/png
Details
FirefoxUX/photon‑extension‑kit pull‑request
(deleted), text/x-github-pull-request
Details
Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves
(deleted), text/x-phabricator-request
mixedpuppy
: review+
Details
Attached file `browser_style` Checkbox Test Extension v1.xpi (obsolete) (deleted) —
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0 Build ID: 20170518000419 Steps to reproduce: Create a simple `options_ui` or `browser/page_action` with `browser_style` set to `true` and the `<body>` element of the `html` file containing the following: ```html <label for="the-checkbox"><input id="the-checkbox" type="checkbox"/>The checkbox</label> ``` Also happens in the following cases: - The `<input>` element is inside the `<label>` element at the end: ```html <label for="the-checkbox">The checkbox<input id="the-checkbox" type="checkbox"/></label> ``` - The `<input>` element is after the `<label>` element: ```html <label for="the-checkbox">The checkbox</label><input id="the-checkbox" type="checkbox"/> ``` - There is no `<label>` element: ```html <input id="the-checkbox" type="checkbox"/>The checkbox ``` - There is no `<label>` element and the `<input>` element is at the end: ```html The checkbox<input id="the-checkbox" type="checkbox"/> ``` This also affects `<input>` elements of type `radio` Actual results: The Checkbox `<input>` element is invisible. Expected results: The Checkbox should have rendered identically to: ```html <input id="the-checkbox" type="checkbox"/><label for="the-checkbox">The checkbox</label> ```
Same problem for radio boxes on Nightly 56.0a1 (2017-06-14) (64-bit) macOS: <label>Proxy server type</label><br> <input name="proxyServerTypeInput" type="radio" value="HTTP" checked>HTTP <input name="proxyServerTypeInput" type="radio" value="SOCKS5">SOCKS5 <input name="proxyServerTypeInput" type="radio" value="HTTPS">HTTPS/SSL I'm using this stylesheet, which is empty: <link rel="stylesheet" href="options.css"/> (zero bytes)
sounds like dupe of similar, matt checking
Flags: needinfo?(mwein)
Depends on: 418833
Assignee: nobody → mstriemer
status-firefox57: --- → fix-optional
Priority: -- → P3
Since bug 418833 got resolved, this can be now be fixed by simply removing `+ label::before` from all `.browser-style > input[type="checkbox"]` and `.browser-style > input[type="radio"]` selectors. Adding as a blocker for bug 1398606 so that this can be done as part of the extension.css Photon update and be released alongside Firefox 60 ESR.
Blocks: 1398606
tracking-firefox60: --- → ?
Flags: needinfo?(ntim.bugs)
I meant bug 1421652.
Blocks: 1421652
No longer blocks: 1398606
I'm concerned about backwards compatibility here, what about people using the old classes/selectors ?
Flags: needinfo?(ntim.bugs)
All this will do is allow <input type="radio"> and <input type="checkbox"> to not require being immediately followed by a <label> that has a [for] attribute pointing at the preceding element without breaking existing setups that do this.
OK, please remind me to do this once bug 1398606 lands. I'll see if I can look into it. I'd prefer not to mix this change with bug 1398606 for clarification purposes.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't understand why tracking was requested here.
tracking-firefox60: ? → ---
Attachment #8951870 - Flags: review?(mstriemer)
Attachment #8951870 - Flags: review?(ntim.bugs)
Congrats on your first patch!
I'll try to review this either tomorrow or Monday.
Comment on attachment 8951870 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves Looks good to me, but a WebExtension peer should review this
Attachment #8951870 - Flags: review?(ntim.bugs)
Attachment #8951870 - Flags: review?(kmaglione+bmo)
Attachment #8951870 - Flags: feedback+
Change the email address used for this commit to match my GitHub account’s commit address.
Attachment #8951870 - Attachment is obsolete: true
Attachment #8951870 - Flags: review?(mstriemer)
Attachment #8951870 - Flags: review?(kmaglione+bmo)
Attachment #8953939 - Flags: review?(mstriemer)
Attachment #8953939 - Flags: review?(kmaglione+bmo)
Summary: browser_style Checkbox is invisible when not in front of it’s associated label element → browser_style Checkbox is invisible when not in front of its associated label element
Reassigning to self
Assignee: mstriemer → e7358d9c
Status: NEW → ASSIGNED
Can you still reproduce this on the latest nightly? The checkboxes and radio buttons appear visible for me.
Flags: needinfo?(matthewjwein) → needinfo?(e7358d9c)
Yes, I can still reproduce this in the lastest nighly, I just forgot to upload the updated testcase to work with the changes made in bug 1354336.
Attachment #8876420 - Attachment is obsolete: true
Flags: needinfo?(e7358d9c)
Flags: needinfo?(mstriemer)
Attached image format-with-patch.png (deleted) —
What is the use case for not having the label after the checkbox or radio button? Since this is using browser styles I think it makes sense to enforce that the form is following our convention of [checkbox/radio] [label]. Here's a screenshot of what I see with this patch applied. It's pretty close but it's just the "Working" case that looks right to me. The others have inconsistent padding and the text is not centred vertically.
Flags: needinfo?(mstriemer) → needinfo?(e7358d9c)
The problem with the current CSS, is that the `input + label::before` CSS is basically a hack, that works around bug 418833. I'm personally ok with this change. The use case is that you may want to lay out the label differently (by putting it beneath the input for example).
Or you might simply not want a label.
Basically what :ntim said. Also because of bug 418833, bug 1354336 made it so that everything has to have the `.browser-style` class applied to use the `chrome://browser/content/extension.css` styles, which resulted in bug 1391464.
Flags: needinfo?(e7358d9c)
Comment on attachment 8953939 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves If you can fix up the padding and vertical alignment that would be good but ideally people should be using the `<input><label>` form anyway so hopefully that doesn't matter much. Getting rid of the ::before hack is good so this seems fine even though it isn't a case we officially support. You'll still need a review from a peer. I know Luca (rpl) has looked at some options_ui stuff as well if Kris isn't free.
Attachment #8953939 - Flags: review?(mstriemer) → review+
Attachment #8953939 - Flags: review?(lgreco)
Comment on attachment 8953939 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves Adding :mixedpuppy as he might also be able to review it.
Attachment #8953939 - Flags: review?(mixedpuppy)
Comment on attachment 8953939 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves This is a big improvement, but I'd still like to see the padding/alignment issues addressed if possible. Might take someone with more css-foo than me to provide the right input for that.
Attachment #8953939 - Flags: review?(mixedpuppy)
Comment on attachment 8953939 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves Hi Blake, can you take a look at this changes to the browser_style css rules to double-check them? (or point out someone that could/should)
Attachment #8953939 - Flags: review?(lgreco) → review?(bwinton)
Comment on attachment 8953939 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves Review of attachment 8953939 [details] [diff] [review]: ----------------------------------------------------------------- I'm happy with the css rule changes. The styling will be changed in bug 1421652, but that's already a separate bug, so we can do any cleanup over there. r=me!
Attachment #8953939 - Flags: review?(bwinton) → review+
Exe Boss, Have you had time to address/reply to comment 24 ?
Flags: needinfo?(e7358d9c)
(In reply to Tim Nguyen :ntim from comment #26) > ExE Boss, Have you had time to address/reply to comment 24? I think you mean comment 25, which I must’ve somehow missed. Anyway, I now need someone to push this to the try server for me, as I don’t yet have push access to it.
Flags: needinfo?(e7358d9c) → needinfo?(ntim.bugs)
ExE Boss, would you like to follow the procedure here to get try commit access ? https://www.mozilla.org/en-US/about/governance/policies/commit/
Flags: needinfo?(ntim.bugs) → needinfo?(e7358d9c)
Considering that I’ll need someone with Level 2 or higher commit access to vouch for me, I expect that I’ll probably first need to have made some decent contributions.
Flags: needinfo?(e7358d9c) → needinfo?(ntim.bugs)
(In reply to ExE Boss from comment #29) > Considering that I’ll need someone with Level 2 or higher commit access to > vouch for me, I expect that I’ll probably first need to have made some > decent contributions. I can vouch for try server access.
Flags: needinfo?(ntim.bugs) → needinfo?(e7358d9c)
(In reply to Tim Nguyen :ntim from comment #30) > (In reply to ExE Boss from comment #29) > > Considering that I’ll need someone with Level 2 or higher commit access to > > vouch for me, I expect that I’ll probably first need to have made some > > decent contributions. > > I can vouch for try server access. I’ve now opened bug 1463449 for this.
Flags: needinfo?(e7358d9c)
Blocks: 1465256
Product: Toolkit → WebExtensions
status-firefox57: fix-optional → ---
Keywords: checkin-needed
Pushed by cbrindusan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ff55a12aca15 Make browser-style checkboxes and radio buttons handle style themselves. r=bwinton,mstriemer
Keywords: checkin-needed
Backed out changeset ff55a12aca15 (bug 1371951) for bc failures in browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js on a CLOSED TREE Problematic push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ff55a12aca154de13e03f6e212f2bd99a8c2d4fb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=188302416 Backout: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2ba85b36cb84008008c0806d585801937e222a73&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Log: https://treeherder.mozilla.org/logviewer.html#?job_id=188302416&repo=mozilla-inbound&lineNumber=1990 00:38:25 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected checkbox item to be hidden - Expected: none, Actual: inline-block - 00:38:25 INFO - Stack trace: 00:38:25 INFO - verifyCheckboxOrRadio@moz-extension://eb99e481-ce34-c34d-a0ab-467f58d4fbe9/options.js:22:11 00:38:25 INFO - @moz-extension://eb99e481-ce34-c34d-a0ab-467f58d4fbe9/options.js:38:7 00:38:25 INFO - 00:38:25 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be visible - 00:38:25 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be visible - 00:38:25 INFO - Not taking screenshot here: see the one that was previously logged 00:38:25 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be hidden - Expected: none, Actual: inline-block - 00:38:25 INFO - Stack trace: 00:38:25 INFO - verifyCheckboxOrRadio@moz-extension://eb99e481-ce34-c34d-a0ab-467f58d4fbe9/options.js:22:11 00:38:25 INFO - @moz-extension://eb99e481-ce34-c34d-a0ab-467f58d4fbe9/options.js:45:7 00:38:25 INFO - 00:38:25 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | options-ui-browser_style - 00:38:25 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | test result correct - 00:38:25 INFO - Leaving test bound test_options_without_setting_browser_style 00:38:25 INFO - Entering test bound test_options_with_browser_style_set_to_true 00:38:25 INFO - Extension loaded 00:38:25 INFO - Console message: Warning: attempting to write 12514 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes. 00:38:25 INFO - Console message: [JavaScript Error: "this.chromeGlobal.content is null" {file: "resource:///modules/ContentLinkHandler.jsm" line: 461}] 00:38:25 INFO - loadIcons@resource:///modules/ContentLinkHandler.jsm:461:9 00:38:25 INFO - ContentLinkHandler/this.iconTask<@resource:///modules/ContentLinkHandler.jsm:457:44 00:38:25 INFO - idleDispatch/</<@resource://gre/modules/PromiseUtils.jsm:40:21 00:38:25 INFO - 00:38:25 INFO - Console message: Warning: attempting to write 12612 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes. 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected correct style when browser_style is set to `true` - 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected correct style when browser_style is set to `true` - Expected: rgb(9, 150, 248), Actual: rgb(9, 150, 248) - 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected checkbox item to be visible - 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected checkbox item to be visible - 00:38:26 INFO - Not taking screenshot here: see the one that was previously logged 00:38:26 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected checkbox item to be hidden - Expected: none, Actual: inline-block - 00:38:26 INFO - Stack trace: 00:38:26 INFO - verifyCheckboxOrRadio@moz-extension://ffa09ebc-b388-2841-95c4-8c5050f899f6/options.js:22:11 00:38:26 INFO - @moz-extension://ffa09ebc-b388-2841-95c4-8c5050f899f6/options.js:38:7 00:38:26 INFO - 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be visible - 00:38:26 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be visible - 00:38:26 INFO - Not taking screenshot here: see the one that was previously logged 00:38:26 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js | Expected radio item to be hidden - Expected: none, Actual: inline-block -
Flags: needinfo?(e7358d9c)
(In reply to Stefan Hindli [:stefan_hindli] from comment #33) > Backed out changeset ff55a12aca15 (bug 1371951) for bc failures in > browser/components/extensions/test/browser/ > browser_ext_optionsPage_browser_style.js on a CLOSED TREE > […] > 00:38:25 INFO - TEST-UNEXPECTED-FAIL | > browser/components/extensions/test/browser/ > browser_ext_optionsPage_browser_style.js | Expected checkbox item to be hidden > - Expected: none, Actual: inline-block - > […] > 00:38:25 INFO - TEST-UNEXPECTED-FAIL | > browser/components/extensions/test/browser/ > browser_ext_optionsPage_browser_style.js | Expected radio item to be hidden > - Expected: none, Actual: inline-block - > […] > 00:38:26 INFO - TEST-UNEXPECTED-FAIL | > browser/components/extensions/test/browser/ > browser_ext_optionsPage_browser_style.js | Expected checkbox item to be hidden > - Expected: none, Actual: inline-block - > […] > 00:38:26 INFO - TEST-UNEXPECTED-FAIL | > browser/components/extensions/test/browser/ > browser_ext_optionsPage_browser_style.js | Expected radio item to be hidden > - Expected: none, Actual: inline-block - Well, that just means that the tests need to be updated (since the <input type="radio"> and <input type="checkbox"> elements are now visible instead of hidden, as they now handle the styling themselves, rather than using the label’s ::before pseudo‑element), but I don’t really know how to do so.
Flags: needinfo?(e7358d9c) → needinfo?(shindli)
I'm not sure this is something I should do.
Flags: needinfo?(shindli) → needinfo?(mstriemer)
This submits the patch to the https://github.com/FirefoxUX/photon-extension-kit repository, which might eventually be used for the extension.css file, similarly to how devtools, Stylo, WebRender and some other components are built into Firefox.
Looks like you can check if it has a background image instead of it being hidden. Can you rebase this patch and verify that it still works as expected, please?
Flags: needinfo?(mstriemer) → needinfo?(e7358d9c)
I’ve rebased and hopefully fixed this now. Also, `background‑image` is only set when the checkbox is checked, so I instead check `background‑color`, like the <button> element style test.
Attachment #8953939 - Attachment is obsolete: true
Attachment #8953939 - Flags: review?(kmaglione+bmo)
Flags: needinfo?(e7358d9c)
Attachment #8997241 - Flags: review?(mstriemer)
You might want to push this patch to try in case you haven't.
(In reply to Tim Nguyen :ntim from comment #39) > You might want to push this patch to try in case you haven't. I don’t really know how to do that.
(In reply to ExE Boss from comment #40) > (In reply to Tim Nguyen :ntim from comment #39) > > You might want to push this patch to try in case you haven't. > > I don’t really know how to do that. Just run: ./mach try -b do -p linux,linux64,macosx64,win32,win64 -u xpcshell,mochitest-bc -t none assuming you have your SSH key setup correctly. Here's the documentation for the try syntax: https://mozilla-releng.net/trychooser/
(In reply to Tim Nguyen :ntim from comment #41) > (In reply to ExE Boss from comment #40) > > (In reply to Tim Nguyen :ntim from comment #39) > > > You might want to push this patch to try in case you haven't. > > > > I don’t really know how to do that. > > Just run: > ./mach try -b do -p linux,linux64,macosx64,win32,win64 -u xpcshell,mochitest-bc -t none > > assuming you have your SSH key setup correctly. > > Here's the documentation for the try syntax: > https://mozilla-releng.net/trychooser/ I tried that, but I’m on Windows, and my PC is having issues, so I need to restart, so could you please do it for me?
Flags: needinfo?(ntim.bugs)
Well, I did at least test my changes locally, so I’m going to try and get this into Firefox 63 and hope that there won’t be any test failures or regressions this time.
tracking-firefox63: --- → ?
Keywords: checkin-needed
(In reply to ExE Boss from comment #43) > Well, I did at least test my changes locally, so I’m going to try and get > this into Firefox 63 and hope that there won’t be any test failures or > regressions this time. The patch does not have a review+, removing checkin needed until then.
Keywords: checkin-needed
Comment on attachment 8997241 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves.patch Requesting review from updated patch from :bwinton
Attachment #8997241 - Flags: review?(bwinton)
Component: Untriaged → Frontend
Comment on attachment 8997241 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves.patch Review of attachment 8997241 [details] [diff] [review]: ----------------------------------------------------------------- Seems fine to me, but I don't think I'm a peer for the WebExtension component, so you should probably get someone else to review it, too. :)
Attachment #8997241 - Flags: review?(bwinton) → review+
Attachment #8997241 - Flags: review?(ntim.bugs)
Comment on attachment 8997241 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves.patch I'm not a WebExtension peer. As for the try push, if you use Phabricator, it's actually much easier to trigger one from there.
Attachment #8997241 - Flags: review?(ntim.bugs) → review?(mixedpuppy)
(In reply to Tim Nguyen :ntim from comment #47) > Comment on attachment 8997241 [details] [diff] [review] > Bug 1371951 - Make browser-style checkboxes and radio buttons handle style > themselves.patch > > I'm not a WebExtension peer. As for the try push, if you use Phabricator, > it's actually much easier to trigger one from there. Well, this patch was submitted before I had configured Phabricator, and I wasn’t able to submit it to MozReview either due to not having a Mozilla IRC account. (Why exactly does MozReview even need that?)
Comment on attachment 8997241 [details] [diff] [review] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves.patch This looks fine. Likely browser style will get an overhaul, but that will not happen in 63.
Attachment #8997241 - Flags: review?(mstriemer)
Attachment #8997241 - Flags: review?(mixedpuppy)
Attachment #8997241 - Flags: review+
So this can finally get merged now.
Flags: needinfo?(ntim.bugs)
Keywords: checkin-needed
I ran browser_ext_optionsPage_browser_style.js locally, and it seems to fail: Unexpected Results ------------------ browser/components/extensions/test/browser/browser_ext_optionsPage_browser_style.js FAIL Expected correct style when browser_style is excluded - Expected: rgb(255, 255, 255), Actual: rgb(9, 150, 248) - Stack trace: verifyCheckboxOrRadio@moz-extension://c25ea5f0-45b2-fd48-8c1d-4d9a8fde44bb/options.js:22:11 @moz-extension://c25ea5f0-45b2-fd48-8c1d-4d9a8fde44bb/options.js:45:7 FAIL Expected correct style when browser_style is set to `true` - Expected: rgb(255, 255, 255), Actual: rgb(9, 150, 248) - Stack trace: verifyCheckboxOrRadio@moz-extension://b19dcbb8-4654-194c-90df-64c55f2cb6ac/options.js:22:11 @moz-extension://b19dcbb8-4654-194c-90df-64c55f2cb6ac/options.js:45:7 browser/components/extensions/test/browser/test-oop-extensions/browser_ext_optionsPage_browser_style.js FAIL Expected correct style when browser_style is excluded - Expected: rgb(255, 255, 255), Actual: rgb(9, 150, 248) - Stack trace: verifyCheckboxOrRadio@moz-extension://cbe81c98-8156-814e-89a4-407deb1a9912/options.js:22:11 @moz-extension://cbe81c98-8156-814e-89a4-407deb1a9912/options.js:45:7 FAIL Expected correct style when browser_style is set to `true` - Expected: rgb(255, 255, 255), Actual: rgb(9, 150, 248) - Stack trace: verifyCheckboxOrRadio@moz-extension://07cb4bca-9bec-ce4f-8fcc-a2676bd6125a/options.js:22:11 @moz-extension://07cb4bca-9bec-ce4f-8fcc-a2676bd6125a/options.js:45:7
Keywords: checkin-needed
This removes the requirement for browser-style checkboxes and radio buttons to be followed by a label since the reason why this was originally done no longer applies because bug 418833 made it possible for checkboxes and radio buttons to handle styling themselves rather than requiring a label::before element.
Attached patch test-fix.diff (obsolete) (deleted) — — Splinter Review
Here's a test fix, just fold it in your last patch, and it should be good to go :)
Flags: needinfo?(e7358d9c)
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e57c44d5ec97 Make browser-style checkboxes and radio buttons handle style themselves. r=mixedpuppy, bwinton
Flags: needinfo?(e7358d9c)
Attachment #8997241 - Attachment is obsolete: true
Comment on attachment 8999189 [details] [diff] [review] test-fix.diff Done
Attachment #8999189 - Attachment is obsolete: true
Thanks! Congrats on landing your first patch btw :)
Depends on: 1482411
Comment on attachment 8999187 [details] Bug 1371951 - Make browser-style checkboxes and radio buttons handle style themselves Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #8999187 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e57c44d5ec97
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Is manual QA needed on this bug? If yes, please provide a test webextension and some STRs, else add the "qe-verify-" flag.
Flags: needinfo?(e7358d9c)
(In reply to Victor Carciu from comment #60) > Is manual QA needed on this bug? If yes, please provide a test webextension > and some STRs, else add the "qe-verify-" flag. I don’t think it is, as the style changes are covered using automated tests, but you can use attachment 8955426 [details] to do a before and after comparison in the options page, which has `<input type="checkbox">` and `<input type="radio">` in all the possible configurations. Without this patch, only the first one works, but with this patch, all of them work correctly (see attachment 8956189 [details]).
Flags: needinfo?(e7358d9c) → qe-verify-
OS: Unspecified → All
Hardware: Unspecified → All
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: