Closed Bug 1216955 Opened 4 years ago Closed 4 years ago

Use chrome override for download-glow-menuPanel-XPVista7.png

Categories

(Firefox :: Theme, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 45
Tracking Status
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: dao, Assigned: harshitbansal2015, Mentored)

References

Details

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

Attachments

(1 file, 2 obsolete files)

Attached patch second.patch (obsolete) — Splinter Review
Kindly review the proposed patch and tell if anything else needs to be done!!
Attachment #8683788 - Flags: review?(dao)
Attachment #8683788 - Flags: checkin?(dao)
Assignee: nobody → harshitbansal2015
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Comment on attachment 8683788 [details] [diff] [review]
second.patch

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

This is the right idea, but...

::: browser/themes/windows/downloads/indicator.css
@@ -65,5 @@
> -@media (-moz-os-version: windows-vista),
> -       (-moz-os-version: windows-win7) {
> -%endif
> -  #downloads-button[cui-areatype="menu-panel"][attention] {
> -    list-style-image: url("chrome://browser/skin/downloads/download-glow-menuPanel-XPVista7.png");

This path:

chrome://browser/skin/downloads/download-glow-menuPanel-XPVista7.png

::: browser/themes/windows/jar.mn
@@ +321,5 @@
>  
>  % override chrome://browser/skin/actionicon-tab.png                   chrome://browser/skin/actionicon-tab-XPVista7.png                 os=WINNT osversion<=6.1
>  % override chrome://browser/skin/reload-stop-go.png                   chrome://browser/skin/reload-stop-go-XPVista7.png                 os=WINNT osversion<=6.1
>  % override chrome://browser/skin/reload-stop-go@2x.png                chrome://browser/skin/reload-stop-go-XPVista7@2x.png              os=WINNT osversion<=6.1
> +% override chrome://browser/skin/download-glow-menuPanel.png          chrome://browser/skin/download-glow-menuPanel.png-XPVista7.png    os=WINNT osversion<=6.1

... doesn't match this path:

chrome://browser/skin/download-glow-menuPanel.png-XPVista7.png

Nor, I think, does the path to the file you're overriding actually work. Did you test this patch? :-)
Attachment #8683788 - Flags: review?(dao) → review-
(Also, please use an appropriate commit message for the next patch you provide. Thank you!)
Flags: needinfo?(jaws)
Attached patch second.patch (obsolete) — Splinter Review
Kindly review the attached patch.
Attachment #8683788 - Attachment is obsolete: true
Attachment #8684081 - Flags: review?(dolske)
Attachment #8684081 - Flags: checkin-
Attachment #8684081 - Flags: checkin- → checkin?(dolske)
Comment on attachment 8684081 [details] [diff] [review]
second.patch

>+% override chrome://browser/skin/downloads/download-glow-menuPanel.png  chrome://browser/skin/downloads/download-glow-menuPanel.png-XPVista7.png os=WINNT osversion<=6.1

chrome://browser/skin/downloads/download-glow-menuPanel.png-XPVista7.png needs to be chrome://browser/skin/downloads/download-glow-menuPanel-XPVista7.png instead.

Also please move this override between the chrome://browser/skin/urlbar-history-dropmarker@2x.png and the chrome://browser/skin/places/autocomplete-star.png overrides.
Attachment #8684081 - Flags: review?(dolske)
Attachment #8684081 - Flags: review-
Attachment #8684081 - Flags: checkin?(dolske)
Attached patch second.patchSplinter Review
Attachment #8684081 - Attachment is obsolete: true
Attachment #8684214 - Flags: review?(dao)
Attachment #8684214 - Flags: checkin?
Comment on attachment 8684214 [details] [diff] [review]
second.patch

Looks good, thanks!
Attachment #8684214 - Flags: review?(dao) → review+
Attachment #8684214 - Flags: checkin?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/17194b40e1df
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Depends on: 1232647
You need to log in before you can comment on or make changes to this bug.