Closed
Bug 1263709
Opened 8 years ago
Closed 8 years ago
New WebExtension panel styles are sometimes a little problematic.
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox48+ fixed, firefox49 fixed)
RESOLVED
FIXED
mozilla49
People
(Reporter: bwinton, Assigned: bwinton)
References
Details
(Keywords: dev-doc-complete, Whiteboard: triaged)
Attachments
(5 files)
49.28 KB,
image/png
|
Details | |
143.20 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
kmag
:
review+
shorlander
:
ui-review+
ritu
:
approval-mozilla-aurora+
|
Details |
743.42 KB,
image/png
|
Details | |
22.37 KB,
image/jpeg
|
Details |
Some people have all their buttons together, and all the labels after them, but would like to not show the button for the last label. Patch written, screenshots coming.
Assignee | ||
Comment 1•8 years ago
|
||
And here's what I've got so far.
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/45787/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45787/
Attachment #8740474 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Updated•8 years ago
|
Attachment #8740474 -
Flags: ui-review?(shorlander)
Comment 3•8 years ago
|
||
Comment on attachment 8740474 [details] MozReview Request: Bug 1263709 - Remove the advanced styling for radio buttons and checkboxes. ui-r=shorlander, r=kmag https://reviewboard.mozilla.org/r/45787/#review42431 ::: browser/components/extensions/extension.css (Diff revision 1) > -input[type="radio"]:checked:hover:active + label::before, > -.radioItem.pressed input[type="radio"]:checked:not(active) + label::before { > - background-color: #005bab; > - border-color: #004480; > -} > - Maybe we should add a class so pages that want these styles can opt into them?
Attachment #8740474 -
Flags: review?(kmaglione+bmo) → review+
Assignee | ||
Comment 4•8 years ago
|
||
I kinda waffled back and forth on that. I eventually went this way because I think we want most WebExtensions to just look good, which implies getting the nicer styles by default. (It also makes it easier to port existing WebExtensions to Firefox.) But I obviously agree to a certain point, since there are some styles which depend on classes (like panel-list-item or panel-section-footer)…
Updated•8 years ago
|
Whiteboard: triaged
Comment 5•8 years ago
|
||
Can it be an option on the manifest? I understand the need to set an UI standard as default, but it would be nice for cross-browser extension to opt-out of it to keep the same style between browsers.
Assignee | ||
Comment 6•8 years ago
|
||
Hi Pedro! What do you think about using a CSS reset (as described at http://meyerweb.com/eric/tools/css/reset/)? Also, (assuming you're writing an add-on) how does it look in Chrome vs. Firefox? (It's not going to be an option on the manifest, but I think we should be able to work out some way to disable it.)
Flags: needinfo?(wachunei)
Comment 7•8 years ago
|
||
Here you can see how style outputs in different browsers.
Flags: needinfo?(wachunei)
Comment 8•8 years ago
|
||
I think it is okay to use a CSS reset when you pretend to override the default style, it is the best option when you want to do that. The point here is that CSS rules are not being applied correctly, my extension is not overriding default styles (you can see that in the options page of Chrome and Opera, select tag is styled according to browser CSS), but I'm using CSS to hide radio inputs (positioning them off viewport), which is working fine in other browsers (and Firefox too) but not working on the first option (SIDING). Here's the CSS rule I'm using to hide those: https://github.com/wachunei/directUC/blob/master/css/popup.css#L141
Assignee | ||
Comment 9•8 years ago
|
||
So, the radio input is exactly what this bug is fixing, as you can see in my screenshot from comment #1. Hopefully it'll land soon!
Comment 10•8 years ago
|
||
You are right, but at the same time it is removing your UI styles for those inputs, when the issue might be a CSS specificity problem.
Comment 11•8 years ago
|
||
(In reply to Blake Winton (:bwinton) (:☕️) from comment #6) > It's not going to be an option on the manifest, but I think we should be > able to work out some way to disable it. I actually think we should probably add a "browser_style" option to match the one in "options_ui".
Comment 12•8 years ago
|
||
It would be great if we could land this before the 48 merge. Otherwise I think we're going to have to uplift it.
Assignee | ||
Comment 13•8 years ago
|
||
kmag: I agree! I got a verbal ui-r+ from shorlander this week, so if he doesn't set the flag tomorrow, I think we would be fine landing it.
Updated•8 years ago
|
Attachment #8740474 -
Flags: ui-review?(shorlander) → ui-review+
Assignee | ||
Comment 14•8 years ago
|
||
https://reviewboard.mozilla.org/r/45787/#review42431 > Maybe we should add a class so pages that want these styles can opt into them? (As mentioned in bugzilla, I think we want styles to apply by default, to make everyone look better.)
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e50f2538de72
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 17•8 years ago
|
||
[Tracking Requested - why for this release]: This makes the styles nicer for add-on authors for the first release of WebExtensions. It's a CSS-only change, and so has little chance of breaking things, and we don't want add-on authors to be frustrated with us, or have to work around this bug for the first release…
tracking-firefox48:
--- → ?
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8740474 [details] MozReview Request: Bug 1263709 - Remove the advanced styling for radio buttons and checkboxes. ui-r=shorlander, r=kmag Approval Request Comment [Feature/regressing bug #]: Fix for problems with bug 1225633. [User impact if declined]: WebExtensions authors will need to work around our styles, and be annoyed by it. [Describe test coverage new/current, TreeHerder]: Manual, style-related. [Risks and why]: Low-risk because it's only removing CSS. [String/UUID change made/needed]: None.
Attachment #8740474 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8740474 [details] MozReview Request: Bug 1263709 - Remove the advanced styling for radio buttons and checkboxes. ui-r=shorlander, r=kmag (Dammit, I should stop doing things before coffee!)
Attachment #8740474 -
Flags: approval-mozilla-beta? → approval-mozilla-aurora?
status-firefox48:
--- → affected
Comment on attachment 8740474 [details] MozReview Request: Bug 1263709 - Remove the advanced styling for radio buttons and checkboxes. ui-r=shorlander, r=kmag Styling usage improvements in webextensions, CSS only so low risk, Aurora48+
Attachment #8740474 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/5eab0f59fd9e
Comment 22•8 years ago
|
||
I’ve intended to verify this bug using the following xpi https://bugzilla.mozilla.org/show_bug.cgi?id=1225633#c35 but it seems to be broken (see the attached screenshot) since Bug 1269081 has landed. Is this webextension outdated and needs to be updated according to Bug 1269081 or this is a regression that require a new bug? Blake, could you please provide me also the other webextension (from the first attachment)?
Flags: needinfo?(bwinton)
Assignee | ||
Comment 23•8 years ago
|
||
Hey Vasilica, the other add-on is at https://github.com/wachunei/directUC (You can download a zip file of it at https://github.com/wachunei/directUC/archive/master.zip, then unzip it, go to "about:debugging", and use the "Load Temporary Add-on" button to load the unzipped directory, by picking any of the files in that directory.) I haven't updated the UI Playground to take advantage of the brower_style yet, so what you're seeing is expected behaviour. I'll probably get around to updating it soon, maybe, but if you need it for testing, I could update it earlier for you. Thanks, Blake.
Flags: needinfo?(bwinton)
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 24•8 years ago
|
||
I've added something on this here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/page_action#Syntax https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/browser_action#Syntax and also: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Anatomy_of_a_WebExtension#Popups Do we need anything else? I'm also wondering, what's the cross-browser story for this? If the CSS classes described here: https://firefoxux.github.io/StyleGuide/#/components/panels are essentially part of the public API, do we expect other browsers to support them, and to apply styles for their own browsers using them? If so, should we have cross-browser docs for them?
Flags: needinfo?(bwinton)
Assignee | ||
Comment 25•8 years ago
|
||
That all looks pretty good. I'm not sure about cross-browser support. That sounds like a question for the people writing the spec… ;)
Flags: needinfo?(bwinton)
Updated•8 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•