1091117 - Search picker/chooser/switcher panel in about:newtab should use @2x DPI image for checkmark glyph/icon

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Drew Willcoxon :adw]

It uses this icon: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/menu-check.png  But there's also a @2x version alongside it that newtab should use on 2x DPI screens.

Better yet, make a single SVG version and use that instead.

Relevant CSS: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css?rev=d3ed7981bd8f#483

This may be fixed as a part of the refactoring done in bug 1029889, but it doesn't have to be.

Comment 1

[Tim Nguyen :ntim]

Here's the 2x file : http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/[e-mail]

Comment 2

[Medhini]

Hi! Its my first time fixing a bug. How do i go about it?

Comment 3

[Tim Nguyen :ntim]

(In reply to Medhini from comment #2)
> Hi! Its my first time fixing a bug. How do i go about it?

Thanks for your interest in this bug !
You need to add hdpi support to [0], using this pattern :
@media (min-resolution: 2dppx) {
[...]
}
In case you don't know how to build firefox yet, you can check [1].

[0] : http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css?rev=d3ed7981bd8f#483
[1] : http://codefirefox.com

Comment 4

[sai sashankh]

hi, Can I also try fixing this bug?

Comment 5

[Avijit Gupta]

hello there, i am interested in fixing this bug.
[i have already set up firefox build environment]

Related to this bug--
I have create a .svg file for the menu-check image, but i am not clear about how to i add it here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css?rev=d3ed7981bd8f#483 (while the existing bug proposes that there should be a single .svg file, but https://bugzilla.mozilla.org/show_bug.cgi?id=1091117#c3 proposes usage of @media tag for differentiating between min-resolution: 2dppx and otherwise).

Comment 6

[Avijit Gupta]

Attachment: a single .svg file instead of 2 different .png files

here is the .svg file i created for the same.

Comment 7

[Tim Nguyen :ntim]

(In reply to Avijit Gupta from comment #6)
> Created attachment 8543259 [details]
> a single .svg file instead of 2 different .png files
> 
> here is the .svg file i created for the same.

That glyph doesn't look exactly look the same. I think we can switch to SVG in another bug, since we don't have an identical asset yet, and because the current asset is also used in other parts of the UI.

Comment 8

[Avijit Gupta]

(In reply to Tim Nguyen [:ntim] from comment #7)

> That glyph doesn't look exactly look the same. I think we can switch to SVG
> in another bug, since we don't have an identical asset yet, and because the
> current asset is also used in other parts of the UI.

does this indicate that the .svg should not be used here?? (instead usage of media tag must be done, as indicated here: https://bugzilla.mozilla.org/show_bug.cgi?id=1091117#c3 ).

Comment 9

[Tim Nguyen :ntim]

(In reply to Avijit Gupta from comment #8)
> (In reply to Tim Nguyen [:ntim] from comment #7)
> 
> > That glyph doesn't look exactly look the same. I think we can switch to SVG
> > in another bug, since we don't have an identical asset yet, and because the
> > current asset is also used in other parts of the UI.
> 
> does this indicate that the .svg should not be used here?? (instead usage of
> media tag must be done, as indicated here:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1091117#c3 ).

Well, if you've got an identical version in SVG, then feel free to replace it, but you'll have to make a bit more changes.

Comment 10

[Avijit Gupta]

Attachment: Modified .svg file

after re-drawing the .svg from scratch, i think this one is very much identical to the original .png file.

so now i assume it would be safe to replace the background url here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css?rev=d3ed7981bd8f#483

Comment 11

[Avijit Gupta]

Attachment: .png vs .svg when viewed side-by-side, at a very high zoom percentage

Screeenshot to show the similarity between the .svg and the .png files

Comment 12

[Tim Nguyen :ntim]

Nice ! Thanks :) 

You should replace the SVG, plus the references to the SVG here : http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check.png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central

Don't forget to remove the @2x file and the occurences to that 2x file here : http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check%402x.png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central

Comment 13

[Avijit Gupta]

(In reply to Tim Nguyen [:ntim] from comment #12)

> You should replace the SVG, plus the references to the SVG here :
> http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check.
> png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central
> 
> Don't forget to remove the @2x file and the occurences to that 2x file here
> :
> http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check%402x.
> png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central

after doing all the changes as per your requirements, i am stuck here:
to make the chrome url for the .svg to work, i have pasted the .svg file (shared-menu-check.svg) at the following path: [0]
(in my local machine)

additionally, i have also pasted shared-menu-check.svg here: [1]

so now, both [0] and [1] contains this additional file:
shared-menu-check.svg
(along with shared-menu-check.png file, which already existed in the both of them)

but when i run `hg status`, it shows me all changed files changed except at [0]. so now i am afraid that if i commit the changes, the chrome url (for the .svg) might not work. please help me out here.

also, i would request this bug to be assigned to me.

[0] : /obj-x86_64-unknown-linux-gnu/dist/bin/chrome/toolkit/skin/classic/global/menu/
[1] : /toolkit/themes/shared/

Comment 14

[Tim Nguyen :ntim]

(In reply to Avijit Gupta from comment #13)
> (In reply to Tim Nguyen [:ntim] from comment #12)
> 
> > You should replace the SVG, plus the references to the SVG here :
> > http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check.
> > png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central
> > 
> > Don't forget to remove the @2x file and the occurences to that 2x file here
> > :
> > http://mxr.mozilla.org/mozilla-central/search?string=shared-menu-check%402x.
> > png&find=%2F&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central
> 
> after doing all the changes as per your requirements, i am stuck here:
> to make the chrome url for the .svg to work, i have pasted the .svg file
> (shared-menu-check.svg) at the following path: [0]
> (in my local machine)
> 
> additionally, i have also pasted shared-menu-check.svg here: [1]
> 
> so now, both [0] and [1] contains this additional file:
> shared-menu-check.svg
> (along with shared-menu-check.png file, which already existed in the both of
> them)
> 
> but when i run `hg status`, it shows me all changed files changed except at
> [0]. so now i am afraid that if i commit the changes, the chrome url (for
> the .svg) might not work. please help me out here.
> 
> also, i would request this bug to be assigned to me.
> 
> [0] :
> /obj-x86_64-unknown-linux-gnu/dist/bin/chrome/toolkit/skin/classic/global/
> menu/
> [1] : /toolkit/themes/shared/
You shouldn't add it to [0], since that folder is your binary. Your problem might be that you haven't replaced properly the jar.mn entries (toolkit/themes/(windows|linux|osx). Btw, you can always drop by #introduction on IRC if you need help.

Comment 15

[Avijit Gupta]

Attachment: Patch - svg referenced in the codebase

Altered references to .png to .svg
Removed @2x.png and all references to the same.

Comment 16

[Tim Nguyen :ntim]

Comment on attachment 8543676 [details] [diff] [review]
Patch - svg referenced in the codebase

Review of attachment 8543676 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good overall. Asking review from someone with review powers :)

::: browser/themes/osx/customizableui/panelUIOverlay.css
@@ -80,5 @@
>      background-size: auto;
>    }
>  
>    .subviewbutton[checked="true"] {
> -    background-image: url("chrome://global/skin/menu/[e-mail]");

The 1x file reference was replaced with the SVG in panelUIOverlay.inc.css.
So this 2x rule can be removed.

::: toolkit/themes/shared/menu-check.svg
@@ +1,1 @@
> +<svg width="32pt" height="32pt" viewBox="0 0 32 32">

32pt should be 32px.
Also, you should add xmlns="http://www.w3.org/2000/svg" as attribute on <svg>. Otherwise, it'll be detected only as a plain xml file by Firefox.

Comment 17

[Avijit Gupta]

Attachment: Patch 2

Removed the 2x rule in ::: browser/themes/osx/customizableui/panelUIOverlay.css
Added xmlns="http://www.w3.org/2000/svg" as attribute on <svg>

Created a new patch accomodating all the changes from the beginning.

Comment 18

[Tim Nguyen :ntim]

Comment on attachment 8543777 [details] [diff] [review]
Patch 2

You can ask for review from someone else if the reviewer takes too much time to get to your review.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Hi Avijit, were you able to get Firefox built with this patch? If so, please attach a screenshot of it to this bug. If you want some help getting your Firefox build up and running, you can join the #introduction channel on IRC (use http://chat.mibbit.com/?server=irc.mozilla.org if you don't have an IRC client).

Comment 20

[Avijit Gupta]

Attachment: Screenshot of successful build with patch applied

Comment 21

[Avijit Gupta]

Attachment: Screenshot showing svg reference in codebase

An instance of the svg file at this place:  (out of the many instances which were replaced)
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css

Comment 22

[Tim Nguyen :ntim]

I think he meant a screenshot of the icon in action (inside the Firefox UI).

Comment 23

[Avijit Gupta]

Attachment: Screenshot of the svg inside the Firefox UI

sorry, i might have misinterpreted this request earlier.

Comment 24

[Tim Nguyen :ntim]

No, I meant a screenshot of the issue fixed inside the Firefox UI.

Anyways, just tested the patch, and it doesn't look right in the new tab page : http://i.imgur.com/KfeIvL7.png

Comment 25

[Avijit Gupta]

> Anyways, just tested the patch, and it doesn't look right in the new tab
> page : http://i.imgur.com/KfeIvL7.png

yeah, it definitely doesn't look right.
i am now working on fixing this.

Comment 26

[Avijit Gupta]

Attachment: Patch - SVG image with the corrected dimesions included

corrected the SVG image.
made a single patch accommodating all the changesets

Comment 27

[Avijit Gupta]

Attachment: MyScreenshot3.png

screenshot showing the modified SVG image after build in the Firefox UI

Comment 28

[:Gijs (he/him)]

Comment on attachment 8553254 [details] [diff] [review]
Patch - SVG image with the corrected dimesions included

Review of attachment 8553254 [details] [diff] [review]:
-----------------------------------------------------------------

In principle this looks right, but the result is still pixelly on a retina screen. Did you actually test the patch on a retina device?

I'm not super sure why it still looks pixelly. How did you create the SVG? Looking at the path, it looks like you're using a bunch of cubic bezier curves to draw straight lines, which doesn't seem like the best approach to me (but I am not an expert on SVG...). I tried changing the width and height parameters to 32px instead, and that made no difference.

Here is some more general feedback about the patch as it stands:

You should also hg rm toolkit/themes/shared/menu-check.png, as nothing uses it now.

::: toolkit/themes/shared/menu-check.svg
@@ +1,1 @@
> +<svg width="16px" height="16px" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">

This should have an XML prolog:

<?xml version="1.0"?>

on the first line.

@@ +1,2 @@
> +<svg width="16px" height="16px" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
> +<path fill="#505459" d=" M24.3 4 C 25.6 5 26.9 6 28.2 7 C 23.5 14 18.8 21 14.1 28 C 10.5 24.4 6.8 20.8 3.2 17.2 C 4.3 16 5.4 14.7 6.4 13.5 C 8.7 15.5 11.1 17.6 13.4 19.6 C 17 14.4 20.6 9.2 24.3 4 Z" />

This should use a system fill color so the checkmark gets the right color depending on OS themes.

Comment 29

[Avijit Gupta]

(In reply to :Gijs Kruitbosch from comment #28)

> In principle this looks right, but the result is still pixelly on a retina
> screen. Did you actually test the patch on a retina device?

No, i did not test it in on a retina screen.
 
> Looking at the path, it looks like you're using a bunch of cubic bezier
> curves to draw straight lines, which doesn't seem like the best approach to
> me (but I am not an expert on SVG...).
yeah, i had made it unnecessarily complex (using bezier curves).
I have created a new svg using simple line paths.

> I tried changing the width and height
> parameters to 32px instead, and that made no difference.

I have made width and height to be 100%, which i think should work now (since the viewbox is set to "0 0 32 32")
 
> You should also hg rm toolkit/themes/shared/menu-check.png, as nothing uses
> it now.
> 
> ::: toolkit/themes/shared/menu-check.svg
> @@ +1,1 @@
> > +<svg width="16px" height="16px" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
> 
> This should have an XML prolog:
> 
> <?xml version="1.0"?>
> 
> on the first line.

my latest patch will accommodate the above changes.
 
> @@ +1,2 @@
> > +<svg width="16px" height="16px" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
> > +<path fill="#505459" d=" M24.3 4 C 25.6 5 26.9 6 28.2 7 C 23.5 14 18.8 21 14.1 28 C 10.5 24.4 6.8 20.8 3.2 17.2 C 4.3 16 5.4 14.7 6.4 13.5 C 8.7 15.5 11.1 17.6 13.4 19.6 C 17 14.4 20.6 9.2 24.3 4 Z" />
> 
> This should use a system fill color so the checkmark gets the right color
> depending on OS themes.

i am not sure about how do i proceed with this one, would this: http://www.w3.org/TR/SVGTiny12/painting.html#systemPaint be helpful?

could you please help me out on this last fix.

Comment 30

[Avijit Gupta]

Attachment: SVG - built using simple line paths instead of bezier curves

Comment 31

[:Gijs (he/him)]

(In reply to Avijit Gupta from comment #29)
> > @@ +1,2 @@
> > > +<svg width="16px" height="16px" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
> > > +<path fill="#505459" d=" M24.3 4 C 25.6 5 26.9 6 28.2 7 C 23.5 14 18.8 21 14.1 28 C 10.5 24.4 6.8 20.8 3.2 17.2 C 4.3 16 5.4 14.7 6.4 13.5 C 8.7 15.5 11.1 17.6 13.4 19.6 C 17 14.4 20.6 9.2 24.3 4 Z" />
> > 
> > This should use a system fill color so the checkmark gets the right color
> > depending on OS themes.
> 
> i am not sure about how do i proceed with this one, would this:
> http://www.w3.org/TR/SVGTiny12/painting.html#systemPaint be helpful?
> 
> could you please help me out on this last fix.


See the CSS declarations at the top of:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/content-contextmenu.svg?raw=1

This SVG is a bit more complicated than the average one, but the concept should be the same - you should be able to set the fill property to the appropriate color by using a CSS selector for the path in your SVG.

As for what the appropriate color is, I wonder if just -moz-use-text-color works. This would require testing - if you're doing it on Ubuntu, you can change the theme to see if that affects the color in the menu panel (I expect the color in about:newtab is hardcoded, but I could be wrong) - if you're using Windows, you can check by using one of the high contrast themes.

If -moz-use-text-color doesn't work, we'll need to do some work to figure out what the right colors are everywhere. I expect 'menutext' will work, but I could be wrong.

Comment 32

[Tim Nguyen :ntim]

Comment on attachment 8555452 [details]
SVG - built using simple line paths instead of bezier curves

Can you provide a screenshot of this asset inside the new tab dropdown menu ?

It looks a bit thinner than the PNG asset in my opinion, but that's fine if the difference is unnoticeable when using the SVG.

Also, I achieved this shape using a polygon :
<svg xmlns="http://www.w3.org/2000/svg" width="32px" height="32px">
	<polygon fill="#505559" points="6,13 13,19 14,20 24,4 28,7 15,27 14,27 3.5,17"></polygon>
</svg>

I have no preference though, feel free to use whatever looks good to you.

Comment 33

[:Gijs (he/him)]

Drew, is this going to have the same issue as the gear icon (bug 1051861) wrt the CSS part and using system colors? If so, maybe we should split that into a separate bug...

Comment 34

[Drew Willcoxon :adw]

The checkmark is already invisible in high-contrast mode as far as I can tell, and it's a giant pain to make things work in high-contrast mode, so I don't think we should try to solve that in this bug.  If it were just a matter of using a system color, sure, we should use a system color -- but like, apparently an SVG with a system color in a background-image is invisible in high-contrast mode.

But having said that, there's no reason not to use a system color, so let's just use GrayText or something.

Comment 35

[Avijit Gupta]

Attachment: SVG - added css, menutext `fill`

added css to set fill according to the theme (menutext).
also, changed the `viewbox` to `0 0 16 16`.

Comment 36

[Avijit Gupta]

also, i have removed `fill` attribute in the <path> element, since it is now being set by the css rule instead.

Comment 37

[Avijit Gupta]

Attachment: Screenshot - svg in new tab dropdown menu

Comment 38

[Avijit Gupta]

(In reply to Avijit Gupta from comment #36)
> also, i have removed `fill` attribute in the <path> element, since it is now
> being set by the css rule instead.

sorry, i have now added a latest attachment with the path fill set to the original color.

Comment 39

[Avijit Gupta]

Attachment: SVG - added css, menutext `fill`

Comment 40

[Tim Nguyen :ntim]

Comment on attachment 8557641 [details]
Screenshot - svg in new tab dropdown menu

(In reply to Avijit Gupta from comment #37)
> Created attachment 8557641 [details]
> Screenshot - svg in new tab dropdown menu

It looks quite different than the original PNG. You should request ui-review if you alter the appearance.
Stephen, what do you think of this asset ? If it doesn't look good to you, can you provide assets ?

Comment 41

[Drew Willcoxon :adw]

This SVG looks strange because the left part is thicker than the right part.  The checkmark should have uniform thickness IMO.

I think maybe we should forget about SVG altogther and just use the @2x PNG. That should make this bug much simpler.  Avijit, can you do that?

Comment 42

[Avijit Gupta]

alright then, i will submit a new patch which uses the @2x PNG.(In reply to Drew Willcoxon :adw from comment #41)

> I think maybe we should forget about SVG altogther and just use the @2x PNG.
> That should make this bug much simpler.  Avijit, can you do that?

okay then, I will submit a new patch accommodating all the changesets.

Comment 43

[Drew Willcoxon :adw]

Thanks!

Comment 44

[Stephen Horlander [:shorlander]]

Attachment: menu-check.svg

Here is an updated checkmark SVG if you want to use it. I don't really have much of a preference on PNG vs SVG here.

Comment 45

[Stephen Horlander [:shorlander]]

Comment on attachment 8557641 [details]
Screenshot - svg in new tab dropdown menu

As mentioned the segments aren't uniform thickness. I attached a different one that is more uniform.

Comment 46

[Avijit Gupta]

Attachment: Patch - used @2x png instead of svg

(as per commment #41 , usage of @2x png has been made instead of svg)

Comment 47

[Tim Nguyen :ntim]

Comment on attachment 8559309 [details] [diff] [review]
Patch - used @2x png instead of svg

Please add a media query instead of replacing directly the current rule (see comment 3) We don't want 1x to show 2x scaled down assets.

Comment 48

[Drew Willcoxon :adw]

Avijit, in addition to what Tim said, here's an example of a media query: http://mxr.mozilla.org/mozilla-central/source/browser/branding/official/content/aboutDialog.css#22

You can see that #leftBox defaults to a background-image of url("chrome://branding/content/about-logo.png"), but then below that, the media query overrides that and sets a new background-image with a @2x PNG.

Comment 49

[Avijit Gupta]

(In reply to Drew Willcoxon :adw from comment #48)
> Avijit, in addition to what Tim said, here's an example of a media query:
> http://mxr.mozilla.org/mozilla-central/source/browser/branding/official/
> content/aboutDialog.css#22

Thanks, your link made my work a lot easier.

i have added the media query below the existing css rule and have been able to successfully build firefox thereafter.
but now i am not sure about how do i see the working @2x png in firefox about:newtab ui. could you please help me with this?

Comment 50

[Tim Nguyen :ntim]

(In reply to Avijit Gupta from comment #49)
> (In reply to Drew Willcoxon :adw from comment #48)
> > Avijit, in addition to what Tim said, here's an example of a media query:
> > http://mxr.mozilla.org/mozilla-central/source/browser/branding/official/
> > content/aboutDialog.css#22
> 
> Thanks, your link made my work a lot easier.
> 
> i have added the media query below the existing css rule and have been able
> to successfully build firefox thereafter.
> but now i am not sure about how do i see the working @2x png in firefox
> about:newtab ui. could you please help me with this?

The best way is to go to about:config and set layout.css.devPixelsPerPx to true. 
You can also zoom to 200%.

Comment 51

[Avijit Gupta]

Attachment: Patch - used @2x png instead of svg

added @2x png through media query, tested on local machine as well.

Comment 52

[Tim Nguyen :ntim]

Comment on attachment 8559849 [details] [diff] [review]
Patch - used @2x png instead of svg

Review of attachment 8559849 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/newtab/newTab.css
@@ +485,5 @@
>  }
>  
>  .newtab-customize-panel-item[selected],
>  .newtab-search-panel-engine[selected] {
>    background: url("chrome://global/skin/menu/shared-menu-check.png") center left 4px no-repeat transparent;

Can this rule be split up ?
background-image: url(...);
background-position: center left 4px;
background-repeat: no-repeat;
background-size: 16px;

@@ +491,5 @@
>  
> +@media (min-resolution: 2dppx) {
> +  .newtab-customize-panel-item[selected],
> +  .newtab-search-panel-engine[selected] {
> +    background: url("chrome://global/skin/menu/[e-mail]") center left 4px no-repeat transparent;

No need to set all the background properties all over again.

We just need to override background-image

@@ +492,5 @@
> +@media (min-resolution: 2dppx) {
> +  .newtab-customize-panel-item[selected],
> +  .newtab-search-panel-engine[selected] {
> +    background: url("chrome://global/skin/menu/[e-mail]") center left 4px no-repeat transparent;
> +    background-size: 16px 16px;

Can the background-size: 16px; rule be moved up to the other ruleset ?

Comment 53

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #50)
> The best way is to go to about:config and set layout.css.devPixelsPerPx to
> true. 
Whoops, meant 2, not true.

Comment 54

[Drew Willcoxon :adw]

Comment on attachment 8559849 [details] [diff] [review]
Patch - used @2x png instead of svg

Review of attachment 8559849 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/newtab/newTab.css
@@ +485,5 @@
>  }
>  
>  .newtab-customize-panel-item[selected],
>  .newtab-search-panel-engine[selected] {
>    background: url("chrome://global/skin/menu/shared-menu-check.png") center left 4px no-repeat transparent;

I'm going to disagree with Tim.  You don't need to split this up because that's existing code and it's outside the scope of this bug.  Let's just get this bug done.

I agree with Tim's other two comments.

Comment 55

[Avijit Gupta]

Attachment: Patch - media query for 2x png

following comment #52 and comment #54, i have updated the patch to meet the requirements.

Comment 56

[Drew Willcoxon :adw]

Comment on attachment 8560017 [details] [diff] [review]
Patch - media query for 2x png

Review of attachment 8560017 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thank you Avijit!  I'll land this later today.

Comment 57

[Drew Willcoxon :adw]

Tree's closed now, so I'll just mark this checkin-needed and let someone else land it whenever it reopens.

No tryserver run necessary since this is just a CSS change.  I manually tested it too.

Comment 58

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/45bc546cb19f

Comment 59

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/45bc546cb19f

Comment 60

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Not something worth additional manual testing it seems. Also tested by Drew in comment 57. I think that should be enough.

Comment 61

[Drew Willcoxon :adw]

Thanks Avijit!

Comment 62

[[e-mail]]

Hy I am new to bug verification! I didn't get how was this working can someone explain me the scenario?

Comment 63

[Jared Wein [:jaws] (please needinfo? me)]

I think this bug is now obsolete since the search picker UI has changed on about:newtab and it no longer includes this checkmark. Drew, can you confirm?

Comment 64

[Jared Wein [:jaws] (please needinfo? me)]

roxrockers, actually, it probably still exists in Firefox 38.

To verify, you should install Firefox 38. Then open a new tab and click on the search dropdown to see the checkmark. It should look crisp. Then go to about:config and change `layout.css.devPixelsPerPx` to `2`. Now open a new tab and try again. Repeat now with `layout.css.devPixelsPerPx` set to `1`.

It should look crisp in all situations. When complete, you can reset the `layout.css.devPixelsPerPx` pref (it defaults to -1.0).

Comment 65

[[e-mail]]

Attachment: Screen Shot 2015-05-26 at 10.41.56 pm.png

with layout.css.devPixelsPerPx set to 1 this was the affect

Comment 66

[Jared Wein [:jaws] (please needinfo? me)]

Thanks for the screenshot. The placement of the popup is certainly wrong, but the rest of the UI looks OK. We can call this bug verified now. Can you file a new bug about the popup being out of place when the devPixelsPerPx doesn't match the platform default?

Comment 67

[[e-mail]]

yeah! Let me file a bug regarding the same thing will You help me out with it! if you can just drop me a mail at [e-mail]