Closed Bug 1426426 Opened 2 years ago Closed Last year

Stop shipping communicator.css

Categories

(Firefox :: Theme, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: Gijs, Assigned: is2ei, Mentored)

References

Details

Attachments

(1 file, 1 obsolete file)

Apparently this is referenced from the xslt-qa stuff, which I'm hoping to remove in bug 1425356, and dom/xslt/tests/XSLTMark/XSLTMark.xul and layout-debug. Pretty sure we can just remove it and/or only ship it with those (test-only) bits if we need to keep it for some reason. Probably worth checking with layout folks.
(When removing it, we should remove the whitelist entry in browser/base/content/test/static/browser_all_files_referenced.js )
Component: General → Themes
Product: Firefox → Toolkit
Priority: -- → P3
Hi I'm Issei.
Could I work on this issue?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Issei Horie [:is2ei] from comment #2)
> Hi I'm Issei.
> Could I work on this issue?

Sure, do you need more help than what's in comment #0? Feel free to ask.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(is2ei.horie)
Attached patch bug-1426426.patch (obsolete) — Splinter Review
Thanks Gijs!

I have questions.
- How can we test the change?
- Do we need to remove https://searchfox.org/mozilla-central/source/browser/themes/linux/communicator/jar.mn#7 too?

I created a patch. Could you check it, please?
Flags: needinfo?(is2ei.horie)
Attachment #8984903 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8984903 [details] [diff] [review]
bug-1426426.patch

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

Hi, yes, you will need to remove the entirety of the `communicator` directory in the 3 os subdirectories (linux, osx, windows) of browser/themes/ . You will also need to remove the 

DIRS += ["communicator"]

lines from all the moz.build files in those browser/themes subdirectories.

Thank you for working on this!

::: dom/xslt/tests/XSLTMark/XSLTMark.xul
@@ -3,4 @@
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
> -<?xml-stylesheet href="chrome://communicator/skin/" type="text/css"?>

This should probably just be replaced with "chrome://global/skin/" as the href.
Attachment #8984903 - Flags: review?(gijskruitbosch+bugs)
Assignee: nobody → is2ei.horie
Mentor: gijskruitbosch+bugs
Status: NEW → ASSIGNED
Component: Themes → Theme
Product: Toolkit → Firefox
Thanks for your comment! I updated the patch.
Could you check it, please?
Attachment #8984903 - Attachment is obsolete: true
Attachment #8985810 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8985810 [details] [diff] [review]
bug-1426426.patch

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

LGTM, thanks!
Attachment #8985810 - Flags: review?(gijskruitbosch+bugs) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1d03f4a4af28
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Depends on: 1477897
You need to log in before you can comment on or make changes to this bug.