As per bug 1196053 comment 31

Do we want this in FF 42, considering the merge from the next week?

Wanted, not strictly required, I think.

Marco, This bug actually makes the security subpanel for a page with Mixed Content look bad, so we may consider moving this to a P1 or P2. ex: see subview here https://people.mozilla.org/~tvyas/mixedcontent.html and here https://people.mozilla.org/~tvyas/mixedboth.html

(In reply to Tanvi Vyas [:tanvi] from comment #3) > Marco, > > This bug actually makes the security subpanel for a page with Mixed Content > look bad, so we may consider moving this to a P1 or P2. > > ex: see subview here https://people.mozilla.org/~tvyas/mixedcontent.html and > here https://people.mozilla.org/~tvyas/mixedboth.html Marking for triage so team can review at daily update meeting.

Based on https://www.dropbox.com/s/z7n0lda2cr5zaci/Screenshot%202015-08-31%2016.32.09.png?dl=0. I assumed the similarity to the "Change Search Settings" button in the search panel was intentional, so I copied the CSS from that.

Top to bottom: default, :hover, :hover:active.

Missed one style, sorry :(

With patch 1.1 applied. Again - top to bottom: default, :hover, :hover:active.

Added a 1em minimum margin above the footer for cases where the body has a lot of content.

Comment on attachment 8717738 [details] [diff] [review] Patch v1.2 Looks good, but Jared will be a better reviewer for the theme changes.

Add rules for :focus, removed a couple of redundant styles.

Comment on attachment 8718081 [details] [diff] [review] Patch v2 Review of attachment 8718081 [details] [diff] [review]: ----------------------------------------------------------------- I had to rebase this patch a small amount in panel.inc.xul, please update rebase your local tree in your next revision. Also, please use mozreview for requesting review as it allows me to expand the diff to get more context. This rule in panel.inc.css is cutting off the bottom-right of the button, #identity-popup-multiView > .panel-viewcontainer > .panel-viewstack > .panel-subviews { background: var(--panel-arrowcontent-background); border-bottom-right-radius: 3.5px; padding: 0; } ::: browser/themes/shared/controlcenter/panel.inc.css @@ +279,5 @@ > + > +#identity-popup-securityView-footer > button { > + -moz-appearance: none; > + margin: 0; > + border-top: 1px solid var(--panel-separator-color); The other borders need to have border-width:0; set on them. The top border of this button doesn't look right. Looking closely it looks like a transparent row of pixels, though they may just be a lot lighter than the button right next to it. See http://screencast.com/t/KPCUGEBC6tP for a screenshot of what this patch looks in my local build. @@ +281,5 @@ > + -moz-appearance: none; > + margin: 0; > + border-top: 1px solid var(--panel-separator-color); > + padding: 8px 20px; > + color: #000; What was wrong with color: ButtonText? On Windows this is using background-color: ThreeDFace because this patch only applies a background-color on the footer, not the <button>. Nonetheless, please keep background-color and color defined next to each other. In this case, you have background-color defined on the #identity-popup-securityView-footer but the color is defined on the <button> child of the footer.

Review commit: https://reviewboard.mozilla.org/r/37285/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37285/

I've tested this patch on Ubuntu and Win8, and the button looks fine to me. (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12) > Also, please use mozreview for > requesting review as it allows me to expand the diff to get more context. First time using MozReview, let me know if I did it right or if there's something I missed. > This rule in panel.inc.css is cutting off the bottom-right of the button, > #identity-popup-multiView > .panel-viewcontainer > .panel-viewstack > > .panel-subviews { > background: var(--panel-arrowcontent-background); > border-bottom-right-radius: 3.5px; > padding: 0; > } Thanks for catching this! The border-radius seems to be an OS X thing (the other panel styles render it visually absent on the other platforms) so I moved it to the OS X specific stylesheet. > ::: browser/themes/shared/controlcenter/panel.inc.css > @@ +279,5 @@ > > + > > +#identity-popup-securityView-footer > button { > > + -moz-appearance: none; > > + margin: 0; > > + border-top: 1px solid var(--panel-separator-color); > > The other borders need to have border-width:0; set on them. > > The top border of this button doesn't look right. Looking closely it looks > like a transparent row of pixels, though they may just be a lot lighter than > the button right next to it. See http://screencast.com/t/KPCUGEBC6tP for a > screenshot of what this patch looks in my local build. I've set border: none; and set the border-top color to #ccc instead of the panel separator color. This value is used elsewhere as well (notably in the search suggestions panel) and contrasts nicely with the footer button. > @@ +281,5 @@ > > + -moz-appearance: none; > > + margin: 0; > > + border-top: 1px solid var(--panel-separator-color); > > + padding: 8px 20px; > > + color: #000; > > What was wrong with color: ButtonText? On Windows this is using > background-color: ThreeDFace because this patch only applies a > background-color on the footer, not the <button>. > > Nonetheless, please keep background-color and color defined next to each > other. In this case, you have background-color defined on the > #identity-popup-securityView-footer but the color is defined on the <button> > child of the footer. Hmm, I copied over the colors from the CSS for the search suggestions panel, and it seems to set the background-color of the button's container, and kept the button transparent. The hover and active colors for the button apply a value with alpha, which effectively darkens the color of the footer underneath. Would you prefer that I set the color of the button directly, instead of having the colors apply over each other? In any case, I thought moz-appearance: none would get rid of any background color, my bad. I've forced it to transparent now.

Comment on attachment 8725042 [details] MozReview Request: Bug 1202280 - [Control Center] Style the security subview's More Information button like a footer. r=jaws https://reviewboard.mozilla.org/r/37285/#review34183 Sorry for the delay on reviewing this, but it looks good :) r=me

https://hg.mozilla.org/integration/fx-team/rev/c36859eaec38c5f0aeb886b97be0950f7a0dd259 Bug 1202280 - [Control Center] Style the security subview's More Information button like a footer. r=jaws

Looks fine, except a small dotted border on Windows - http://i.imgur.com/Og1AavU.png. Verified fixed FX 47.0a1 (2016-03-06) Win 7, Ubuntu 14.04, OS X 10.10.5. Let me know if you want the above issue filed separately.

Hmm, did we not want the focus ring? (In reply to Paul Silaghi, QA [:pauly] from comment #18) > Let me know if you want the above issue filed separately. Yeah, that would be great, thanks!