Patch only adds bottom border to sidebar header and adjusts spacing (based on interactions in other sidebar bugs, I haven't included any changes to the highlight style).

(In reply to Nihanth Subramanya [:nhnt11] from comment #2) > Patch only adds bottom border to sidebar header and adjusts spacing (based > on interactions in other sidebar bugs, I haven't included any changes to the > highlight style). Are you going to file another bug?

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review166704 ::: browser/themes/windows/browser-aero.css:19 (Diff revision 1) > @media (-moz-windows-default-theme) { > .sidebar-header, > #sidebar-header { > -moz-appearance: none; > - border-bottom: none; > + border-bottom: 1px solid hsla(240, 5%, 5%, .1); > } Can we just get rid of this rule if we remove: http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/toolkit/themes/windows/global/global.css#141-143 ?

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review167744

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review174864 ::: browser/themes/windows/browser.css:904 (Diff revision 2) > #sidebar { > background-color: Window; > } > > #sidebar-header { > - border-bottom: 1px solid ThreeDLightShadow; > + border-bottom: 1px solid hsla(240, 5%, 5%, .1); I suspect the difference between this custom color and ThreeDLightShadow is rather small. Can you confirm? If so, I think we should just keep ThreeDLightShadow. I expect that the hsla value won't work for high contrast themes.

(In reply to Dão Gottwald [::dao] from comment #7) > Comment on attachment 8890280 [details] > Bug 1384504 - [Photon] Update sidebar tree styling on Windows. > > https://reviewboard.mozilla.org/r/161406/#review174864 > > ::: browser/themes/windows/browser.css:904 > (Diff revision 2) > > #sidebar { > > background-color: Window; > > } > > > > #sidebar-header { > > - border-bottom: 1px solid ThreeDLightShadow; > > + border-bottom: 1px solid hsla(240, 5%, 5%, .1); > > I suspect the difference between this custom color and ThreeDLightShadow is > rather small. Can you confirm? If so, I think we should just keep > ThreeDLightShadow. I expect that the hsla value won't work for high contrast > themes. Yeah, it's not a huge difference. I reverted it. I also changed the background color of the sidebar to white in this patch, seems like it fits in the scope of this bug.

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review174900 ::: browser/themes/windows/browser.css:900 (Diff revision 3) > /* ::::: content area ::::: */ > > %include ../shared/sidebar.inc.css > > -#sidebar { > - background-color: Window; > +#sidebar-box { > + background-color: -moz-Field; Should also set -moz-fieldtext as the text color. And it looks like we'll need this on Linux too... ::: browser/themes/windows/places/places.css:8 (Diff revision 3) > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > /* Sidebars */ > + > +:root { > + background-color: transparent; I don't think transparency works here, you're probably just getting the same white default background as web content this way.

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review174900 > Should also set -moz-fieldtext as the text color. > > And it looks like we'll need this on Linux too... I'll file another bug for Linux. > I don't think transparency works here, you're probably just getting the same white default background as web content this way. Nah, I tested by changing the background of #sidebar-box to red; it seems to work.

(In reply to Nihanth Subramanya [:nhnt11] from comment #12) > I'll file another bug for Linux. Filed bug 1392222.

Comment on attachment 8890280 [details] Bug 1384504 - [Photon] Update sidebar tree styling on Windows. https://reviewboard.mozilla.org/r/161406/#review175916

Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3c5b27abffa7 [Photon] Update sidebar tree styling on Windows. r=dao

Hi Nihanth, what do we need to verify here? Thanks

(In reply to ovidiu boca[:Ovidiu] from comment #17) > Hi Nihanth, what do we need to verify here? Thanks This patch updates the styling of the sidebar header, the background color of the sidebar, and the spacing within the bookmarks/history list in the sidebar (all on Windows).

Hi Nihanth, We did a comparison between the expected results from the spec: https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/229252085 and the actual results on the latest Nightly 57.0a1(2017-08-24) and here are the results: 1. The text from "Bookmarks" has a bigger font than the one from spec 2. The colors from the close button "x", "Bookmarks" text the star and the arrow that is near to "Bookmarks" are different from the spec, in spec from what I see the colors are grayer. 3. The distance between search box and "Bookmarks" box is different from the one that is in the spec. 4. In the History sidebar, an extra button "view" appears. These tests were made on Windows 10.

(In reply to ovidiu boca[:Ovidiu] from comment #19) > Hi Nihanth, > We did a comparison between the expected results from the spec: > https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/229252085 and the > actual results on the latest Nightly 57.0a1(2017-08-24) and here are the > results: > > 1. The text from "Bookmarks" has a bigger font than the one from spec We discussed this and we're leaving it large, see http://design.firefox.com/people/shorlander/photon/Mockups/windows-10.html > 2. The colors from the close button "x", "Bookmarks" text the star and the > arrow that is near to "Bookmarks" are different from the spec, in spec from > what I see the colors are grayer. The close button shares its style with the tab close button, so I think it's correct. As for the star and the arrow, I'll ask shorlander about this. > 3. The distance between search box and "Bookmarks" box is different from the > one that is in the spec. Noted. Not sure if we should add the extra padding. I'll ask shorlander and Dao. > 4. In the History sidebar, an extra button "view" appears. Yes, this is missing from the spec. > These tests were made on Windows 10. Thanks!

Hi Nihanth, any news about this? Thanks.

The issues presented at 1,2 and 4 are ok. The only question remains at point 3.

We encounter another issue on Windows 7 x32 where the buttons from sidebar are different from the specifications. Please see the attached print screen with the actual result.

I verify this issue on Windows 8.1 x64 where the buttons from sidebar are different from the specifications. Please see the attachment.

Reproducible on Ubuntu 16.04 x64.

Other than matching Stephen's html mock, we should update the button style for the dropdown button (History pane) to look like it does on macOS (label + dropdown arrow and a standard Photon hover style).

I verify this issue and for issue 3, from comment #19, I logged in bug 1400165. Considering than issue 3 will be tracked in bug 1400165 and 1,2 and 4 are clarified, I will marked this verified.

based on the fact that this issue is still reproducible in Ubuntu, I 've logged bug 1400266.