Our form elements look pretty out of date, so we should update them to better fit with our new design aesthetic. (https://twitter.com/cwiiis/status/211573240836005888) This is the bug to track that progress, and we should include * pulldown menus * input fields * text fields * radio buttons * checkboxes I'll post some new designs soon.

New designs! http://cl.ly/image/441g2z2X3p0r

Mostly there, but still some annoying issues with the file upload. I'll post a screenshot/build and get some feedback from ian and mark.

Adds prefs to disable drawing checkmarks or radio buttons in layout. I think I need to name these better, but wanted to make sure this approach was ok with dbaron. In the long run, I've got a WIP at implementing nsNativeTheme for Android, but its quite a ways from being done and this seems useful for now.

Build is at: http://dl.dropbox.com/u/72157/fennec-forms.apk

My test page is at http://dl.dropbox.com/u/72157/forms.html

Omg. This looks excellent. Let's ship it! \o/

Actually... Mark just made a good point, the disabled text input looks very similar to disabled buttons. I wonder if the disabled input fields can be tweaked somewhat, perhaps with a white background and light grey text / border

Comment on attachment 720974 [details] [diff] [review] Patch 1/2 >diff --git a/mobile/android/themes/core/content.css b/mobile/android/themes/core/content.css >+input[type="submit"]::-moz-focus-inner, >+input[type="file"] > input[type="button"]::-moz-focus-inner { >+ padding: 0; >+ border: none; > } > >-xul|window xul|scrollbarbutton[sbattr="scrollbar-up-top"], >-xul|window xul|scrollbarbutton[sbattr="scrollbar-bottom-top"] { >- display: none; >+ >+button:disabled:active, button:disabled, Looks like we end up with two blank lines here >diff --git a/mobile/android/themes/core/images/icons.svg b/mobile/android/themes/core/images/icons.svg Can we rename to form-icons.svg ? >+.outline { >+ fill:none; >+ stroke:#777980; >+ stroke-width:2.7; >+ stroke-miterlimit:4; Use space after ':' >+.fill { >+ fill:url(#fillGradient); >+ fill-opacity:1; >+ stroke:none; Same >+#checkbox-fill { >+ fill: url(#fillGradient); >+ stroke:#ffffff; >+ stroke-width:1.7; >+ stroke-linecap:round; >+ stroke-linejoin:round; >+ stroke-miterlimit:4; And so on >+ <path d="m 6,10 l 8.5,8.75 l 8.5,-8.75" >+ style="fill:none;stroke:url(#fillGradient);stroke-width:5;stroke-linecap:round;stroke-linejoin:round;stroke-miterlimit:4;"/> And so forth Visually, my only feedback is what Ian mentioned: disabled textboxes and normal buttons might be too similar

Finally tracked my file input woes to: http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsFileControlFrame.cpp#598 which depends on styles in forms.css to be true. Fixing....

Hows this ian? I also increased the strength of gradient up the buttons a bit (maybe too much).

grrr. gradients.

(In reply to Wesley Johnston (:wesj) from comment #10) > Hows this ian? I also increased the strength of gradient up the buttons a > bit (maybe too much). I think the button styles were fine the way you had them before, tbh. The new disabled fields look great, and the disabled text color on buttons is great. I would just revert the button backgrounds back to the version before this and we're good to go :)

(In reply to Ian Barlow (:ibarlow) from comment #12) > (In reply to Wesley Johnston (:wesj) from comment #10) > > Hows this ian? I also increased the strength of gradient up the buttons a > > bit (maybe too much). > > I think the button styles were fine the way you had them before, tbh. The > new disabled fields look great, and the disabled text color on buttons is > great. I would just revert the button backgrounds back to the version before > this and we're good to go :) Agreed

CSS changes. Moved back to the old button style and tweaked the dropmarker and checkboxes a little bit. One thing I haven't put in the screenshots because its kinda hard are active and hover states. I'll upload a build just in case ian wants to test them. I also updated my test page to include a second set of form elements with a scoped stylesheet applied. We've got parity with desktop there and things look ok (even with a pretty ugly theme).

This adds the prefs I needed. If you know of a better solution, happy to hear!

Comment on attachment 722892 [details] [diff] [review] Patch 1/2 - Layout prefs Dholbert, if you're comfortable reviewing thins, feel free to steal.

Comment on attachment 722892 [details] [diff] [review] Patch 1/2 - Layout prefs Review of attachment 722892 [details] [diff] [review]: ----------------------------------------------------------------- Talked to dholbert about this in person. The issue I'm worried about here is that this will expose some theming abilities to content that they didn't have before. Namely, the ability to theme checkboxes or radios. I think that would make webdevs happy, but I'm sure there is a reason we didn't grant it before? If we are going to expose it, maybe a better way is to just check if the element has a background image set, and if so don't draw the checkmark? dholbert's comment was that we should just implement nsNativeTheme which I'm working on, but I'm not convinced that we want all of it anyway. Android theme elements were not designed to blend well with the web. It will be interesting to see which ones work and which don't.

(In reply to Wesley Johnston (:wesj) from comment #17) > dholbert's comment was that we should just implement nsNativeTheme which I'm > working on, but I'm not convinced that we want all of it anyway. Android > theme elements were not designed to blend well with the web. It will be > interesting to see which ones work and which don't. Yeah I'm a little skeptical of how well that will work out. Open to seeing some screenshots, but if it makes the web look broken we shouldn't take that approach.

(Take my comments with a grain of salt: I haven't been involved with the design/bootstrapping of any other native themes, so I'm not sure what's involved with that.) To expand slightly on wesj's paraphrased version of my comments: I'm wondering why we've never done anything like this for other platforms, and why android needs to be a special-case here. This feels like it could be a slippery slope towards a situation where we've got N platforms, some subset of which have a "true" native theme, some subset of which use this background hack & C++-special-casing for some of its form controls, and some subset of which just use the cross-platform theme. That's a complex landscape, and it seems like it'd be cleaner & more future-proof to make this fit into our existing framework. (wesj said that there might be issues with getting actual remote android widgets into gecko, due to how android is set up. If that ends up being a barrier, I suspect we could implement a "native" theme that actually just draws static images, under the hood.)

This is almost the same, but I ripped out the checkbox/radio/file input stuff for something much simpler. This doesn't override the layout styling and looks ok. There's a small issue with the file input controls that I can't quite pin down... I also added some styling for input[type="range"] to get the ball rolling there. I'll post an updated screenshot.

Oh yeah, I restyled the select elements to be a bit more androidy too (without going to far overboard.

Comment on attachment 728498 [details] [diff] [review] Updated patch again >diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js >+pref("layout.forms.radio.draw", false); >+pref("layout.forms.checkmarks.draw", false); >+pref("layout.forms.fileinput.draw.border", true); Do you still need these? What's up with the radios and checkboxes in the screenshot? The ones in the "black background" section are not sized correctly. I am not too sure about the range thumb styling. I am glad you added it, but let's get some UX feedback on those and the <select> drop arrow buttons. Otherwise looks good. I want to withhold r+ until we get an answer on the prefs and an OK on the styling from UX

Also, given this is the last week of the Fx22 cycle, I think we should land these in Fx23.

(In reply to Mark Finkle (:mfinkle) from comment #23) > Do you still need these? No. will Remove. Thanks > What's up with the radios and checkboxes in the screenshot? The ones in the > "black background" section are not sized correctly. I have a input { width: 90%; } style rule in there which makes those do that. > I am not too sure about the range thumb styling. I am glad you added it, but > let's get some UX feedback on those and the <select> drop arrow buttons. I agree. I thought for a first draft I'd do something sorta Android like, but I"m not sure it fits will with these form elements. Likewise, after seeing it on a few sites, I'm still debating whether I love that new "select" style either.

(In reply to Mark Finkle (:mfinkle) from comment #24) > Also, given this is the last week of the Fx22 cycle, I think we should land > these in Fx23. How will this work for <input type=range>? Can you guys decide on an interim design for Fx22 while these new designs are worked out? The other option is to flip the pref to turn off <input type=range> (dom.experimental_forms_range), but you'll also need to remove the applicable styling from forms.css since that styling isn't (can't be) behind that pref.

(In reply to Jonathan Watt [:jwatt] from comment #26) > (In reply to Mark Finkle (:mfinkle) from comment #24) > > Also, given this is the last week of the Fx22 cycle, I think we should land > > these in Fx23. > > How will this work for <input type=range>? Can you guys decide on an interim > design for Fx22 while these new designs are worked out? The other option is > to flip the pref to turn off <input type=range> > (dom.experimental_forms_range), but you'll also need to remove the > applicable styling from forms.css since that styling isn't (can't be) behind > that pref. I think we could land a "just <input type='range'>" patch for Fx22

(In reply to Mark Finkle (:mfinkle) from comment #27) > I think we could land a "just <input type='range'>" patch for Fx22 No need. <input type=range> was disabled in v22.

Comment on attachment 722892 [details] [diff] [review] Patch 1/2 - Layout prefs My gut feeling is that these should be system metrics on the nsITheme implementation rather than prefs. But maybe there's a reason that doesn't work? (Sorry for taking so long to get to this.)

Sorry this languished for awhile. I've given up on themeing the checkboxes or radios in this bug then. This instead just focuses on the inputs and button elements. I also dropped the padding on them by default, as some sites (I ran into Google Maps) depend on inputs having zero (or very little) padding. I also left styling range elements out. I think we need some UX love there before we move on. I'll post a screenshot

Also, note the appearance of input[type="file"] changed recently. This is updated for that.

Comment on attachment 763897 [details] [diff] [review] Patch I'm willing to give this a try. Let's land it soon, so we get decent coverage in Fx25. I assume Ian is OK with the look and feel.

Ian - Looking for a thumbs up/down from you

Let's land it. I have feedback but we can address it in follow up bugs, this is still a huge improvement over what we have.

I was about to push and thought there might be some reftests that don't like this. Turns out, I was right: https://tbpl.mozilla.org/?tree=Try&rev=78ff3ba25b14 working through them...

Just to update. Some of these reftests look like: <input type="text> matches <input type="text" style="padding: 0;"> With this patch, they don't. I'd love to just disable them, but I think webcontent probably relies on this behaviour. After talking to dbaron, he tells me there isn't a good standard for native theming in this sense. For instance, should: <input type="text" style="padding: 10px;"> add padding on top of the native padding, or should it replace the native padding? The testcase above would indicate that Gecko wants to enforce the former. I think maybe(?) if I implement some of this in nsINativeTheme we can work around these issues, so I've been looking at that, but it requires we do the drawing by hand in Thebes (i.e. setting any native theme properties overrides everything in content.css).

In the mean time, we've gotten some complaints from frameworks that our current base theme is hard to deal with. Maybe we can implement something thats similar, but with less padding here.

TBH, I don't have time to look at this.

Want to pick this up bug again. Where can I find a link to a comprehensive list of all the form elements I need to design for?

Sure. Here's a page with all of them. Some are also listed here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input?redirectlocale=en-US&redirectslug=HTML%2FElement%2FInput This page doesn't quite have every disabled state in it, but most of them are the same as something else. We CAN'T do whatever you dream of here very easily. i.e. We can basically only set borders and background-colors. Maybe border-radius. Anything else (images, padding, gradients, shadows, etc) is unlikely to be overridden by sites, and as such will likely break things. We may be able to play with "inner" pieces of the widgets though. Things like the "Browse" button in File inputs, the little buttons to go up or down in number inputs. The arrow in <select> boxes. No custom radio or checkboxes though. I'd like to do this in two phases if we can. One set of simple changes, and a larger bug for big changes (that will require writing some C++ code to draw widgets by hand).

I gave this a quick whirl and wanted to post a WIP to see if this stuff was possible. I've kept it all pretty basic to say the least. Essentially I wanted to figure out a "quick win" approach and then go from there. Thoughts?

Thanks Anthony. I believe this bug is also about taking a look at things like basic text buttons, radios, checkboxes, sliders, and a few others. Check out Wes's test page of _all the things_ https://dl.dropboxusercontent.com/u/72157/forms.html

Short attempt at this. The mockups have more padding than we will probably be able to make. Moving the "Browse..." button on File inputs to the right didn't want to work for me. Not sure why, but it may just be impossible. We could probably localize it differently for mobile though. I'm not sure if we can replace it with an icon (for accessibility reasons). Will have to make sure that an aria label is set, even if there is not button text. I can replace the <select> arrows with the up-down ones if you have graphics (SVG is what we use there so that these look nice scaled). The magnifying glass for search boxes is probably something we shouldn't do either. No one else does by default, so we're likely to conflict with sites own styles. Will play with ellipsizing and try to figure out why its not happening. Build at: http://people.mozilla.com/~wjohnston/forms.apk Lots of polish left though. Always lots of polish left to do. And I just guessed on the widgets not in your picture.

(In reply to Wesley Johnston (:wesj) from comment #47) > Created attachment 8453316 [details] > Screenshot Wow. This is mostly wonderful. My only nit: The disabled "Browse..." button does not line up with the normal ones. +1 on the SVG up/downs and not using the search image.

I think this feels pretty good. I made the svg myself. dholbert told me performance should be pretty good like this. I also moved us to use css variables :) We don't have to do that..

Updated with progress and meter elements. They aren't technically form elements, but they're good enough for me.

And a pretty picture.

Staring at that screenshot, I noticed the weird brown in disabled radios and checkboxes, as well as on option elements. Updated to force them to use our normal colors.

Comment on attachment 8458340 [details] [diff] [review] Patch >diff --git a/mobile/android/themes/core/images/arrows.svg b/mobile/android/themes/core/images/arrows.svg >+<!-- Created with Inkscape (http://www.inkscape.org/) --> Needed? >+ <style type="text/css"> >+.arrow { >+ stroke-width: 1px; >+ stroke-linecap: round; >+ stroke-linejoin: round; >+} >+ >+.sprite { >+ display: none; >+ fill: black; >+ stroke: black; >+} >+ >+.sprite:target { >+ display: block; >+} >+ >+.disabled { >+ fill: gray; >+ stroke: gray; >+} >+ </style> Does it matter if we indent the style section? Let's wait to land this next week for Fx34 and give us time to look for problems. We can up lift if things look good.

(In reply to Wesley Johnston (:wesj) from comment #46) > Created attachment 8453313 [details] [diff] [review] > WIP v1 > > Short attempt at this. The mockups have more padding than we will probably > be able to make. Moving the "Browse..." button on File inputs to the right > didn't want to work for me. Not sure why, but it may just be impossible. We > could probably localize it differently for mobile though. I'm not sure if we > can replace it with an icon (for accessibility reasons). Will have to make > sure that an aria label is set, even if there is not button text. Hm, could you expand on the limitations we're working with here a little more? Maybe we could figure out other ways to give the form elements a little bit more breathing room. > I can replace the <select> arrows with the up-down ones if you have graphics > (SVG is what we use there so that these look nice scaled). I have an .svg file I could attach to this bug. > The magnifying glass for search boxes is probably something we shouldn't do > either. No one else does by default, so we're likely to conflict with sites > own styles. Indeed, I just kept those in there because I wanted to show how they'd line up to our current fields that I've been using mostly in my designs. > Lots of polish left though. Always lots of polish left to do. And I just > guessed on the widgets not in your picture.

Here are .SVG versions of the select icon on dropdowns. Let me know if these help!

updates our Android 2.3 style form elements to a more modern look

> Hm, could you expand on the limitations we're working with here a little > more? Maybe we could figure out other ways to give the form elements a > little bit more breathing room. I landed this, but we can continue to tweak. I suspect (but am not 100% sure) that adding horizontal padding will break some tests. We could disable those, but I've also seen a lot of sites that depend on inputs have 0 horizontal padding by default for their layouts. Adding some on can cause them to break badly. Vertical padding is less likely to cause problems. > I have an .svg file I could attach to this bug. Cool. The file I wound up using is... special :) in that I traced your original and then hand edited the svg into a set of sprites, all using one arrow, translated and scaled into different positions. I looked these over and I think we're pretty close, but if you want, I can incorporate these arrows in it instead.

sorry had to back this out for failing android reftests like https://tbpl.mozilla.org/php/getParsedLog.php?id=44490191&tree=Fx-Team

Argh. Sorry. I pushed to try last week, but apparently didn't check the results. Just to show how great these reftests are, we apparently have a test that: <div style="padding: 0; margin: 0; border: none; background: transparent; -moz-appearance: none; width: 0; height: 0; font: inherit; padding: 1px 3px"> Some text <div style="background: green; width: 100px; height: 100px"></div> </div> and <button style="padding: 0; margin: 0; border: none; background: transparent; -moz-appearance: none; width: 0; height: 0; font: inherit; color: black"> Some text <div style="background: green; width: 100px; height: 100px"></div> </button> both render the same. i.e. a button has to be the same as a div with some random padding added to it. And that has to be true because webkit did that at some time in the past. Yay! I'll dig through these and try to figure out what we need to make them pass... probably in a second patch. Alternatively, we could disable these tests on Android and turn them on one by one...

np :) btw i noticed in the tbpl test results there is also https://tbpl.mozilla.org/php/getParsedLog.php?id=44488740&tree=Fx-Team (just mentioning because this seems to be different failures than the others) 20:55:36 INFO - 2883 INFO TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_bug657143.html | SVG property list should be alphabetical - got "clip-path,clip-rule,color-interpolation,color-interpolation-filters,dominant-baseline,fill,fill-opacity,fill-rule,filter,flood-color,flood-opacity,image-rendering,lighting-color,marker-end,marker-mid,marker-start,mask,mask-type,paint-order,shape-rendering,stop-color,stop-opacity,stroke,stroke-dasharray,stroke-dashoffset,stroke-linecap,stroke-linejoin,stroke-miterlimit,stroke-opacity,stroke-width,text-anchor,text-rendering,vector-effect,moz-form-textcolor,moz-form-border-focused,moz-form-background-disabled,moz-form-border-invalid,moz-highlight-color,moz-form-button-background,moz-range-progress-disabled,moz-form-textcolor-placeholder,moz-form-border,moz-form-border-radius,moz-form-textcolor-disabled", expected "clip-path,clip-rule,color-interpolation,color-interpolation-filters,dominant-baseline,fill,fill-opacity,fill-rule,filter,flood-color,flood-opacity,image-rendering,lighting-color,marker-end,marker-mid,marker-start,mask,mask-type,moz-form-background-disabled,moz-form-border,moz-form-border-focused,moz-form-border-invalid,moz-form-border-radius,moz-form-button-background,moz-form-textcolor,moz-form-textcolor-disabled,moz-form-textcolor-placeholder,moz-highlight-color,moz-range-progress-disabled,paint-order,shape-rendering,stop-color,stop-opacity,stroke,stroke-dasharray,stroke-dashoffset,stroke-linecap,stroke-linejoin,stroke-miterlimit,stroke-opacity,stroke-width,text-anchor,text-rendering,vector-effect"

(In reply to Wesley Johnston (:wesj) from comment #58) > I landed this, but we can continue to tweak. I suspect (but am not 100% > sure) that adding horizontal padding will break some tests. We could disable > those, but I've also seen a lot of sites that depend on inputs have 0 > horizontal padding by default for their layouts. Adding some on can cause > them to break badly. Vertical padding is less likely to cause problems. Good point. I'll keep this in mind as a I keep iterating on this.

(In reply to Carsten Book [:Tomcat] from comment #61) > btw i noticed in the tbpl test results there is also > https://tbpl.mozilla.org/php/getParsedLog.php?id=44488740&tree=Fx-Team > 20:55:36 INFO - 2883 INFO TEST-UNEXPECTED-FAIL | > /tests/layout/style/test/test_bug657143.html | SVG property list should be > alphabetical This looks to me like a bug in the test, apparently caused by adding CSS variables to a UA stylesheet. I filed bug 1043461 on it.

David, do you think it is OK that the CSS variables defined in the UA style sheet are exposed to authors? In general, values set by the UA style sheet are visible to authors, but the use of variables seems like an implementation detail. I'm not sure there's a good way to hide them, though.

It seems bad. The question I'm not sure about is how bad. I see two main problems at first glance: (1) It seems bad that we're exposing these particular CSS variables as an API to the Web for authors to use to change the styles of Fennec form elements, adding yet another set of unstandardized stuff to the story of form element styling. (2) It's not clear to me whether the form controls were designed under the threat model that these variables could be under hostile control; it seems like it might need security review.

Could we have the variables set on elements in the shadow trees rather than on the <input> etc. elements themselves?

(BTW, I talked about wesj about the variable-leakage (and the fact that authors can override the variable values) in MV today -- he's OK with removing the variables (via preprocessing) if they're problematic. Preprocessing seems like a better solution here to me, unless/until we want to explicitly expose these colors as things that authors can use & override for site-theming purposes.)

Working my way through these. A small amount of them are actually tests that are fixed by the new styling. The new bustage mostly came from using an outline on invalid form field rather than box-shadow. I'm probably going to just switch back to box-shadow to avoid breaking sites. A list of tests: PASS | box-shadow/611574-1.html - removing background image makes this pass PASS | box-shadow/611574-2.html - removing background image makes this pass PASS | layout/reftests/bugs/424074-1.xul - scrollbars fixed this FAIL | layout/reftests/bugs/491180-1 - inner focus ring spacing is required FAIL | layout/reftests/bugs/491180-1 - inner focus ring spacing is required PASS | layout/reftests/bugs/513318-2.xul - This test expects to fail because of the scrollbar. Old fennec hid them in xul. I removed that. PASS | layout/reftests/bugs/560455-1.xul - Removing background image fixes it FAIL | layout/reftests/bugs/966992-1.html - Our input[type=submit/reset/etc] padding rules are more specific than the tests input ones... I'm not sure how to fix this or why it didn't fail before. PASS | layout/reftests/css-disabled/button/button-fieldset-2.html - :disabled vs [disabled] styling PASS | layout/reftests/css-disabled/button/button-fieldset-3.html - :disabled vs [disabled] styling PASS | layout/reftests/css-disabled/button/button-fieldset-legend-2.html - :disabled vs [disabled] styling PASS | layout/reftests/css-disabled/button/button-fieldset-legend-3.html - :disabled vs [disabled] styling FAIL | layout/reftests/css-disabled/select/select-fieldset-4.html - :disabled on select button, border shouldn't be there... PASS | layout/reftests/css-enabled/button/button-fieldset-2.html - :disabled vs [disabled] styling PASS | layout/reftests/css-enabled/button/button-fieldset-3.html - :disabled vs [disabled] styling PASS | layout/reftests/css-enabled/button/button-fieldset-legend-2.html - :disabled vs [disabled] styling PASS | layout/reftests/css-enabled/button/button-fieldset-legend-3.html - :disabled vs [disabled] styling FAIL | layout/reftests/css-enabled/select/select-fieldset-4.html - :disabled on select button, border shouldn't be there... FAIL | layout/reftests/css-required/css-required-text.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-file.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-password.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-tel.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-search.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-dyn-1.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-dyn-3.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-required/css-required-dyn-5.html - test expects only a box-shadow change for invalid forms FAIL | layout/reftests/css-valid/input/input-customerror.html - test expects only a box-shadow change for invalid forms PASS | layout/reftests/css-valid/input/input-disabled.html - Changed styling to make our rules less specific PASS | layout/reftests/css-valid/input/input-dyn-disabled.html - Changed styling to make our rules less specific FAIL | layout/reftests/css-valid/input/input-required-invalid.html - test expects only a box-shadow change FAIL | layout/reftests/css-valid/input/input-email-invalid.html - test expects only a box-shadow change FAIL | layout/reftests/css-valid/input/input-url-invalid.html - test expects only a box-shadow change FAIL | layout/reftests/css-valid/input/input-pattern-invalid.html - test expects only a box-shadow change FAIL | layout/reftests/css-valid/input/input-type-invalid.html - test expects only a box-shadow change PASS | layout/reftests/css-valid/textarea/textarea-disabled.html - background-image fixed it PASS | layout/reftests/css-valid/textarea/textarea-dyn-disabled.html - background-image fixed it PASS | layout/reftests/forms/text-control-baseline-1.html - fix by removing background-image FAIL | layout/reftests/forms/button/max-height.html - fixed by removing padding on buttons FAIL | layout/reftests/forms/button/percent-height-child-1.html - fixed by removing forced padding FAIL | layout/reftests/forms/button/percent-height-child-2.html - fixed by removing forced padding FAIL | layout/reftests/forms/button/percent-width-child-1.html - fixed by removing forced padding FAIL | layout/reftests/forms/button/percent-width-child-2.html - fixed by removing forced padding FAIL | layout/reftests/forms/button/vertical-centering.html FAIL | layout/reftests/forms/placeholder/placeholder-6-textarea.html FAIL | layout/reftests/forms/input/number/number-same-as-text-unthemed.html FAIL | layout/reftests/forms/input/number/number-similar-to-text-unthemed.html FAIL | layout/reftests/forms/input/number/show-value.html FAIL | layout/reftests/forms/input/number/number-disabled.html FAIL | layout/reftests/forms/input/number/number-auto-width-1.html FAIL | layout/reftests/forms/input/number/focus-handling.html FAIL | layout/reftests/forms/input/number/number-selected.html FAIL | layout/reftests/forms/input/range/direction-unthemed-1.html PASS | layout/reftests/forms/input/range/moz-range-progress-1.html FAIL | layout/reftests/forms/input/range/moz-range-progress-3.html PASS | layout/reftests/forms/input/text/size-2.html FAIL | layout/reftests/forms/meter/values.html FAIL | layout/reftests/forms/meter/values-rtl.html FAIL | layout/reftests/forms/meter/margin-padding.html FAIL | layout/reftests/forms/meter/margin-padding-rtl.html FAIL | layout/reftests/forms/meter/bar-pseudo-element.html FAIL | layout/reftests/forms/meter/bar-pseudo-element-rtl.html FAIL | layout/reftests/forms/meter/values-vertical.html FAIL | layout/reftests/forms/meter/values-vertical-rtl.html FAIL | layout/reftests/forms/meter/margin-padding-vertical.html FAIL | layout/reftests/forms/meter/margin-padding-vertical-rtl.html FAIL | layout/reftests/forms/meter/bar-pseudo-element-vertical.html FAIL | layout/reftests/forms/meter/bar-pseudo-element-vertical-rtl.html FAIL | layout/reftests/forms/meter/default-style/default-style.html FAIL | layout/reftests/forms/meter/default-style/default-style-dyn.html FAIL | layout/reftests/forms/progress/values.html FAIL | layout/reftests/forms/progress/values-rtl.html FAIL | layout/reftests/forms/progress/margin-padding.html FAIL | layout/reftests/forms/progress/margin-padding-rtl.html FAIL | layout/reftests/forms/progress/bar-pseudo-element.html FAIL | layout/reftests/forms/progress/bar-pseudo-element-rtl.html FAIL | layout/reftests/forms/progress/indeterminate-style-width.html FAIL | layout/reftests/forms/progress/values-vertical.html FAIL | layout/reftests/forms/progress/values-vertical-rtl.html FAIL | layout/reftests/forms/progress/margin-padding-vertical.html FAIL | layout/reftests/forms/progress/margin-padding-vertical-rtl.html FAIL | layout/reftests/forms/progress/bar-pseudo-element-vertical.html FAIL | layout/reftests/forms/progress/bar-pseudo-element-vertical-rtl.html FAIL | layout/reftests/forms/progress/indeterminate-style-height.html PASS | layout/reftests/forms/textarea/resize.html PASS | layout/reftests/forms/textarea/ltr.html PASS | layout/reftests/forms/textarea/ltr.html PASS | layout/reftests/forms/textarea/rtl.html PASS | layout/reftests/forms/textbox/setsize.xul PASS | layout/reftests/native-theme/native-theme-disabled-cascade-levels.html FAIL | layout/reftests/svg/foreignObject-form-theme.svg FAIL | widget/reftests/progressbar-fallback-default-style.html FAIL | widget/reftests/meter-fallback-default-style.html FAIL | layout/xul/grid/reftests/scrollable-columns.xul FAIL | content/html/content/reftests/autofocus/input-number.html FAIL | layout/style/test/test_value_storage.html

Trying again: https://tbpl.mozilla.org/?tree=Try&rev=d63b59bb3638 This is pretty drastically pulled back. Just removes gradients and replaces them with colors from this bug. And updates borders and border-radius values. I expect to see some tests start to pass and show up as failures.

All of the failures in that push were tests that started passing. Fixed and repushed: https://tbpl.mozilla.org/?tree=Try&rev=67c639e5a958

Looks good on try!

Comment on attachment 8481539 [details] [diff] [review] Patch I think it's best to wait for Fx35 and then uplift if we want to and if the risk is low enough.

I scoped this back to get it landed. This just removed the gradient (even from radio buttons and checkboxes), and updated the border-color/radius and text colors. I'll file separate bugs for other things I'd like to do.

I scoped this back to get it landed. This just removed the gradient (even from radio buttons and checkboxes), and updated the border-color/radius and text colors. I'll file separate bugs for other things I'd like to do.

Comment on attachment 8481539 [details] [diff] [review] Patch I'm going to nom this to go all the way to beta. We've hated these forever. Its just taken awhile to fix them. They're a big thorn in our mobile web compat side. Approval Request Comment [Feature/regressing bug #]: forever [User impact if declined]: ugly forms [Describe test coverage new/current, TBPL]: this actually made a lot of tests pass on tbpl :) there are tons of reftets for these things. [Risks and why]: low risk. affects default form display of websites, but should bring it back in line with desktop/other browsers. [String/UUID change made/needed]: none.

(In reply to Wesley Johnston (:wesj) from comment #78) > I'm going to nom this to go all the way to beta. We've hated these forever. > Its just taken awhile to fix them. They're a big thorn in our mobile web > compat side. Can you expand on how this impacts web compat?

AFAIK, we're the only mobile browser that has a "default" background-image on form elements. Since no one else does, sites often don't add background-image: none; when styling form elements resulting in a different appearance in Firefox. This removes that background-image entirely and brings us more in line with other browsers/webdev expectations for default styles.

The main thing this fixes is that sites that use a button for their main navigation no longer have a gradient. Compare a medium.com [1] article in release and nightly for example. This affects the M in the upper right of the page because they use a button element for the navigation. [1] https://medium.com/stuff-and-more-stuff/making-a-unicorn-tear-dc25dd9a1c79

Comment on attachment 8481539 [details] [diff] [review] Patch Thanks Wes and Kevin. That is pretty visible. As the patch looks safe, let's take this for mobile beta4. Beta+ and Aurora+

https://hg.mozilla.org/releases/mozilla-aurora/rev/cf27552884dd https://hg.mozilla.org/releases/mozilla-beta/rev/7b4d4b3b7598

Added to the release notes with "Form elements updated to a more modern look" as wording. Don't hesitate if you have a better wording to suggest!