Closed Bug 1091117 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 38
Iteration:
38.2 - 9 Feb
Tracking Status
firefox38 --- fixed

People

(Reporter: adw, Assigned: 526avijit, Mentored)

References

Details

(Whiteboard: [good first bug][lang=css])

Attachments

(3 files, 16 obsolete files)

548 bytes, image/svg+xml
Details
1.09 KB, patch
adw
: review+
Details | Diff | Splinter Review
90.81 KB, image/png
Details
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.
Flags: qe-verify+
Flags: firefox-backlog+
Mentor: felipc
Whiteboard: [good first bug][lang=css]
Hi! Its my first time fixing a bug. How do i go about it?
(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
hi, Can I also try fixing this bug?
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).
here is the .svg file i created for the same.
(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.
(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 ).
(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.
Attached image Modified .svg file (obsolete) —
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
Attachment #8543259 - Attachment is obsolete: true
Screeenshot to show the similarity between the .svg and the .png files
(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/
(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.
Assignee: nobody → 526avijitgupta
Status: NEW → ASSIGNED
Mentor: felipc → ntim007
Altered references to .png to .svg
Removed @2x.png and all references to the same.
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/shared-menu-check@2x.png");

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.
Attachment #8543676 - Flags: review?(felipc)
Attached patch Patch 2 (obsolete) — Splinter Review
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.
Attachment #8543777 - Flags: review?(felipc)
Attachment #8543676 - Attachment is obsolete: true
Attachment #8543676 - Flags: review?(felipc)
Flags: needinfo?(ntim007)
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.
Flags: needinfo?(ntim007)
Attachment #8543777 - Flags: review?(felipc) → review?(jaws)
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).
Flags: needinfo?(526avijitgupta)
Flags: needinfo?(526avijitgupta)
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
I think he meant a screenshot of the icon in action (inside the Firefox UI).
Flags: needinfo?(526avijitgupta)
sorry, i might have misinterpreted this request earlier.
Attachment #8552461 - Attachment is obsolete: true
Attachment #8552463 - Attachment is obsolete: true
Flags: needinfo?(526avijitgupta)
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
Attachment #8543777 - Flags: review?(jaws)
> 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.
corrected the SVG image.
made a single patch accommodating all the changesets
Attachment #8543518 - Attachment is obsolete: true
Attachment #8543777 - Attachment is obsolete: true
Flags: needinfo?(ntim007)
Attached image MyScreenshot3.png (obsolete) —
screenshot showing the modified SVG image after build in the Firefox UI
Attachment #8552628 - Attachment is obsolete: true
Flags: needinfo?(ntim007)
Attachment #8553254 - Flags: review?(jaws)
Flags: needinfo?(ntim007)
Flags: needinfo?(ntim007) → needinfo?(jaws)
Flags: needinfo?(jaws)
Attachment #8553254 - Flags: review?(jaws) → review?(gijskruitbosch+bugs)
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.
Attachment #8553254 - Flags: review?(gijskruitbosch+bugs) → review-
(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.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8555452 - Flags: review?(ntim007)
(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.
Flags: needinfo?(gijskruitbosch+bugs)
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.
Attachment #8555452 - Flags: review?(ntim007)
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...
Flags: needinfo?(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.
Flags: needinfo?(adw)
Mentor: adw
Attached image SVG - added css, menutext `fill` (obsolete) —
added css to set fill according to the theme (menutext).
also, changed the `viewbox` to `0 0 16 16`.
Attachment #8553254 - Attachment is obsolete: true
Flags: needinfo?(adw)
also, i have removed `fill` attribute in the <path> element, since it is now being set by the css rule instead.
Attachment #8553258 - Attachment is obsolete: true
Flags: needinfo?(ntim007)
(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.
Attached image SVG - added css, menutext `fill` (obsolete) —
Attachment #8555452 - Attachment is obsolete: true
Attachment #8557639 - Attachment is obsolete: true
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 ?
Flags: needinfo?(ntim007)
Attachment #8557641 - Flags: ui-review?(shorlander)
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?
Flags: needinfo?(adw)
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.
Thanks!
Attached image 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 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.
Attachment #8557641 - Flags: ui-review?(shorlander) → ui-review-
(as per commment #41 , usage of @2x png has been made instead of svg)
Attachment #8559309 - Flags: review?(gijskruitbosch+bugs)
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.
Attachment #8559309 - Flags: review?(gijskruitbosch+bugs)
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.
(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?
Flags: needinfo?(adw)
(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%.
Flags: needinfo?(adw)
added @2x png through media query, tested on local machine as well.
Attachment #8543519 - Attachment is obsolete: true
Attachment #8557641 - Attachment is obsolete: true
Attachment #8557644 - Attachment is obsolete: true
Attachment #8559309 - Attachment is obsolete: true
Flags: needinfo?(ntim007)
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/shared-menu-check@2x.png") 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/shared-menu-check@2x.png") center left 4px no-repeat transparent;
> +    background-size: 16px 16px;

Can the background-size: 16px; rule be moved up to the other ruleset ?
Flags: needinfo?(ntim007)
(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 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.
Attachment #8559849 - Flags: feedback+
following comment #52 and comment #54, i have updated the patch to meet the requirements.
Attachment #8559849 - Attachment is obsolete: true
Attachment #8560017 - Flags: review?(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.
Attachment #8560017 - Flags: review?(adw) → review+
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.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/45bc546cb19f
Keywords: checkin-needed
Whiteboard: [good first bug][lang=css] → [good first bug][lang=css][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/45bc546cb19f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=css][fixed-in-fx-team] → [good first bug][lang=css]
Target Milestone: --- → Firefox 38
Iteration: --- → 38.2 - 9 Feb
Not something worth additional manual testing it seems. Also tested by Drew in comment 57. I think that should be enough.
Flags: qe-verify+ → qe-verify-
Thanks Avijit!
Hy I am new to bug verification! I didn't get how was this working can someone explain me the scenario?
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?
Flags: needinfo?(adw)
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).
Flags: needinfo?(adw)
with layout.css.devPixelsPerPx set to 1 this was the affect
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?
Status: RESOLVED → VERIFIED
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 roxrockers@gmail.com
You need to log in before you can comment on or make changes to this bug.