Move the current Preferences Applications pane to an in-content applications UI

Moved the applications pane to in-content UI. Only changes I made were in the applications.xul file changing the wrapping box. *Note: unsure about the correctness of the way I styled the applications pane for getting overflow to work.

Comment on attachment 603378 [details] [diff] [review] in-content applications pane Review of attachment 603378 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/preferences/in-content/applications.xul @@ +39,5 @@ > +# the terms of any one of the MPL, the GPL or the LGPL. > +# > +# ***** END LICENSE BLOCK ***** > +--> > +<vbox id="applications-content" hidden="true"> We should remove this <vbox> and place a <vbox> around each preference as part of the search implementation. ::: browser/components/preferences/in-content/in-content.css @@ +187,4 @@ > color: #444; > text-shadow: 0 0 3px white; > background: -moz-linear-gradient(rgba(251,252,253,0.95), rgba(246,247,248,0) 49%, > + rgba(211,212,213,0.45) 51%, rgba(225,226,229, 0.3)); nit: Can you fix the indentation here to match the opening parenthesis of the -moz-linear-gradient( ? @@ +236,4 @@ > > #encrytionPanel { > > +} Can you remove the empty #encryptionPanel rule?

Fixed all issues addressed in Jared's feedback.

Comment on attachment 605536 [details] [diff] [review] applications in-content pane Review of attachment 605536 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/preferences/in-content/applications.js @@ +39,5 @@ > +# and other provisions required by the GPL or the LGPL. If you do not delete > +# the provisions above, a recipient may use your version of this file under > +# the terms of any one of the MPL, the GPL or the LGPL. > +# > +# ***** END LICENSE BLOCK ***** This should be MPL2. @@ +53,5 @@ > +var Ci = Components.interfaces; > +var Cr = Components.results; > +/* > +#endif > +*/ These will need to be removed after the feedback is addressed for bug 733473. ::: browser/components/preferences/in-content/applications.xul @@ +38,5 @@ > +# the provisions above, a recipient may use your version of this file under > +# the terms of any one of the MPL, the GPL or the LGPL. > +# > +# ***** END LICENSE BLOCK ***** > +--> MPL2 here too. ::: browser/components/preferences/in-content/in-content.css @@ +182,4 @@ > color: #444; > text-shadow: 0 0 3px white; > background: -moz-linear-gradient(rgba(251,252,253,0.95), rgba(246,247,248,0) 49%, > + rgba(211,212,213,0.45) 51%, rgba(225,226,229, 0.3)); Please fix the indentation here.

fixed all feedback from Jared, and fixed indentation

Comment on attachment 608497 [details] [diff] [review] applications in-content pane Review of attachment 608497 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/winstripe/preferences/in-content/preferences.css @@ +107,5 @@ > > +/* Applications Pane Styles */ > + > +#applications-content { > + padding: 15px; Is this padding duplicated for other categories too?

no, it is specifically used for the box in applications

fixed jar file spacing

added consistent title

included search vbox's, and fixed title

Comment on attachment 609224 [details] [diff] [review] applications in-content pane Review of attachment 609224 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/preferences/in-content/applications.js @@ +7,5 @@ > + > +/* > +#ifndef XP_MACOSX > +*/ > +var Cr = Components.results; IIRC, this is an old hack that's no longer needed. Remove it all (comment markers, #ifdef, Components.results). And add Components.results to preferences.js, same as Components.classes et al. ::: browser/components/preferences/in-content/applications.xul @@ +59,5 @@ > + > +<keyset> > + <key key="&focusSearch1.key;" modifiers="accel" oncommand="gApplicationsPane.focusFilterBox();"/> > + <key key="&focusSearch2.key;" modifiers="accel" oncommand="gApplicationsPane.focusFilterBox();"/> > +</keyset> Nit: Move this <keyset> to above the <vbox> header (we generally try to group non-visible elements together at the top). @@ +72,5 @@ > +</vbox> > + > +<separator class="thin"/> > + > +<vbox data-category="applications" hidden="true"> This and the <seperator> and the textbox above need to all be inside the same <vbox> (you can't use the filter without the list of things that it filters). ::: browser/themes/gnomestripe/preferences/in-content/preferences.css @@ +98,5 @@ > + padding: 15px; > +} > + > +#handlersView { > + border: 1px solid rgba(31,64,100,0.4); Use ThreeDLightShadow as the color here (on gnomestripe, we try to blend in with the system theme, so we usually can't use custom colours). @@ +99,5 @@ > +} > + > +#handlersView { > + border: 1px solid rgba(31,64,100,0.4); > + height: 500px; We generally don't use 'px' units when dealing with sizes that aren't for cosmetic purposes - 'em' is generally more appropriate. About 30em seems right here (applies to all themes). ::: browser/themes/winstripe/preferences/in-content/preferences.css @@ +94,5 @@ > + padding: 15px; > +} > + > +#handlersView { > + border: 1px solid rgba(31,64,100,0.4); I assume you got this colour from the winstripe theme - you'll need to find the relevant colour from the pinstripe theme (it's different).

removed vbox from heading, fixed blairs feedback, removed vbox around 1st child elements and added data-category and hidden to 1st child element instead

Comment on attachment 614534 [details] [diff] [review] applications in-content patch for new landing patch Review of attachment 614534 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/gnomestripe/preferences/in-content/preferences.css @@ +111,5 @@ > +} > + > +#handlersView { > + border: 1px solid ThreeDLightShadow; > + height: 30em; Looking at this, it doesn't take up much space in a big window. It should really scale in such cases. So change this to be max-height (same with the other themes), and add flex="1" to the <richlistbox> and it's parent <vbox>. That should make it adjust better to both large and small windows, and for when there are multiple things showing (when we get proper text search working). ::: browser/themes/pinstripe/preferences/in-content/preferences.css @@ +113,5 @@ > + padding: 15px; > +} > + > +#handlersView { > + border: 1px solid ThreeDShadow; The colour here should match what's used in the pinstripe theme - rgba(60,73,97,0.5) Also, for the border colour to have any affect, you need to add the following (applies to gnomestripe and winstripe as well): -moz-appearance: none;

removed the height style completely, why waste available space?

I verify fixed this bug, tested at Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120621 Firefox/15.0a2.