Closed Bug 1426426 Opened 4 years ago Closed 4 years ago

Stop shipping communicator.css


(Firefox :: Theme, enhancement, P3)




Firefox 62
Tracking Status
firefox62 --- fixed


(Reporter: Gijs, Assigned: is2ei, Mentored)




(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 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]

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 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 -->
> -<?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
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]

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

LGTM, thanks!
Attachment #8985810 - Flags: review?(gijskruitbosch+bugs) → review+
Keywords: checkin-needed
Pushed by
Stop shipping communicator.css. r=gijs
Keywords: checkin-needed
Closed: 4 years ago
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.