Closed Bug 1280135 Opened 7 years ago Closed 7 years ago

The search indicator in Linux uses a low rez version on hidpi displays

Categories

(Firefox :: Search, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
firefox50 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The very attractive search indicator icon is fuzzy in Linux!

There isn't a good reason for this since the images are not platform dependent, so it is just a linux css theme tweak.
Comment on attachment 8762771 [details]
Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.

https://reviewboard.mozilla.org/r/59262/#review56310

Annnnd you can guess my feedback by now... can we unify this? Doesn't feel like it makes sense to duplicate this CSS 3 times over. Please request the next review for this particular patch from :florian.
Attachment #8762771 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #2)
> Comment on attachment 8762771 [details]
> Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.
> 
> https://reviewboard.mozilla.org/r/59262/#review56310
> 
> Annnnd you can guess my feedback by now... can we unify this?

Theoretically, yes? There is no shared css for this piece. Maybe there should be becuase there is a lot of style in common across the 3 platforms. But I think it is beyond the scope of this bug.
Comment on attachment 8762771 [details]
Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59262/diff/1-2/
Attachment #8762771 - Flags: review?(florian)
(In reply to Eitan Isaacson [:eeejay] from comment #3)

> > Annnnd you can guess my feedback by now... can we unify this?
> 
> Theoretically, yes? There is no shared css for this piece. Maybe there
> should be becuase there is a lot of style in common across the 3 platforms.
> But I think it is beyond the scope of this bug.

There was some attempt to unify in bug 1106569, but it never got finished because that bug ended up mixing refactoring/merging with an actual bugfix and it got confusing. Yes, this duplicated code should be unified, but let's do this change in its own bug.
Attachment #8762771 - Flags: review?(florian)
Comment on attachment 8762771 [details]
Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.

https://reviewboard.mozilla.org/r/59262/#review57054

Looks good to me by code inspection (I don't have access to a hidpi Linux machine), just one comment that I think should be addressed.

::: browser/themes/linux/searchbar.css:89
(Diff revision 2)
>  
>  .searchbar-search-button:hover:active {
>    -moz-image-region: rect(0, 60px, 20px, 40px);
>  }
>  
> +@media (min-resolution: 1.1dppx) {

It looks to me like you also want to include in your patch this section of the Windows CSS file:
http://searchfox.org/mozilla-central/source/browser/themes/windows/searchbar.css#249
Comment on attachment 8762771 [details]
Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59262/diff/2-3/
Attachment #8762771 - Flags: review?(florian)
Comment on attachment 8762771 [details]
Bug 1280135 - Make hidpi version of search-indicator icon work on Linux.

https://reviewboard.mozilla.org/r/59262/#review59216
Attachment #8762771 - Flags: review?(florian) → review+
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/450e5d0b3df0
Make hidpi version of search-indicator icon work on Linux. r=florian
https://hg.mozilla.org/mozilla-central/rev/450e5d0b3df0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.