Closed Bug 344987 Opened 18 years ago Closed 18 years ago

Remove remaining xpi detritus from various places

Categories

(Calendar :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mattwillis, Assigned: mattwillis)

Details

Attachments

(3 files)

There's still some xpi gack remaining in various places that should be removed.
In addition these files will be removed:
/mozilla/calendar/createBuildId.pl
/mozilla/calendar/xpi/*

/mozilla/calendar/resources/content/alertDialog.xul
/mozilla/calendar/resources/content/calendarOverlay.js
/mozilla/calendar/resources/content/calendarOverlay.xul
/mozilla/calendar/resources/skin/classic/alertDialog.css
/mozilla/calendar/resources/skin/classic/btn1.png
/mozilla/calendar/resources/skin/classic/calendar.css
/mozilla/calendar/resources/skin/classic/calendar-16.gif
/mozilla/calendar/resources/skin/classic/calendar_topbar.gif
/mozilla/calendar/resources/skin/classic/datetimepickers/calendar_down.gif
/mozilla/calendar/resources/skin/classic/datetimepickers/calendar_hover.gif
/mozilla/calendar/resources/skin/classic/datetimepickers/clock_down.gif
/mozilla/calendar/resources/skin/classic/datetimepickers/clock_hover.gif
/mozilla/calendar/resources/skin/classic/datetimepickers/datepicker.css
/mozilla/calendar/resources/skin/classic/datetimepickers/timepicker.css
/mozilla/calendar/resources/skin/classic/eventDialog.css
/mozilla/calendar/resources/skin/classic/overlay.css
/mozilla/calendar/resources/skin/classic/pageupdown.png
/mozilla/calendar/resources/skin/classic/prevnextarrow.png
/mozilla/calendar/resources/skin/classic/selectAddresses.css
/mozilla/calendar/resources/skin/classic/taskbar-cal-act.gif
/mozilla/calendar/resources/skin/classic/taskbar-cal.gif
/mozilla/calendar/resources/skin/classic/taskbar-calalarm-act.gif
/mozilla/calendar/resources/skin/classic/taskbar-calalarm.gif
Attachment #229583 - Flags: first-review?(jminta)
Summary: Remove remaining xpi detrius from various places → Remove remaining xpi detritus from various places
Comment on attachment 229583 [details] [diff] [review]
rev0 - stops packaging unused files 

http://lxr.mozilla.org/mozilla/source/xpfe/components/xremote/src/XRemoteService.cpp#775
Someone should probably talk to the seamonkey folks about that too.

-#expand  skin/classic/calendar/calendar-views.css	(/calendar/sunbird/themes/__THEME__/sunbird/calendar-views.css)
+#expand  skin/classic/calendar/calendar.css      (/calendar/sunbird/themes/__THEME__/sunbird/calendar.css)
 #expand  skin/classic/calendar/prevnextarrow.png	(/calendar/sunbird/themes/__THEME__/sunbird/prevnextarrow.png)
 #expand  skin/classic/calendar/pageupdown.png	(/calendar/sunbird/themes/__THEME__/sunbird/pageupdown.png)
 #expand  skin/classic/calendar/calendar-toolbar.css	(/calendar/base/themes/__THEME__/calendar-toolbar.css)
 #expand  skin/classic/calendar/calendar-views.css	(/calendar/base/themes/__THEME__/calendar-views.css)

This looks wrong.  Why does lightning need Sunbird's calenadr.css.  It's been getting buy without it so far.

-#ifndef MOZ_SUNBIRD
-    skin/classic/calendar/calendar.css (skin/classic/calendar.css)
-    skin/classic/calendar/prevnextarrow.png (skin/classic/prevnextarrow.png)
-    skin/classic/calendar/datetimepickers/minimonth.css (skin/classic/datetimepickers/minimonth.css)
-#endif
+% skin calendar classic/1.0 %skin/classic/calendar/
The + line looks beyond the scope of this bug (removing detritus), can you explain what it's doing here?

+    skin/classic/calendar/unifinder/priority_low.png (skin/classic/unifinder/priority_low.png)
+    skin/classic/calendar/week-view-corner.png (skin/classic/week-view-corner.png)

Same here.  What's needed about week-view-corner that wasn't needed before?

I'll review the actual removal list once we get this patch sorted out.
(In reply to comment #2)
> (From update of attachment 229583 [details] [diff] [review] [edit])
> http://lxr.mozilla.org/mozilla/source/xpfe/components/xremote/src/XRemoteService.cpp#775
> Someone should probably talk to the seamonkey folks about that too.

Also if you do text and filename searches on calendar-window you'll find more goodies. Since that requires review from non-calendar folk, I left it out of this patch, but did plan to address it in this bug.

> -#expand  skin/classic/calendar/calendar-views.css     
> (/calendar/sunbird/themes/__THEME__/sunbird/calendar-views.css)
> +#expand  skin/classic/calendar/calendar.css     
> (/calendar/sunbird/themes/__THEME__/sunbird/calendar.css)
>  #expand  skin/classic/calendar/prevnextarrow.png      
> (/calendar/sunbird/themes/__THEME__/sunbird/prevnextarrow.png)
>  #expand  skin/classic/calendar/pageupdown.png 
> (/calendar/sunbird/themes/__THEME__/sunbird/pageupdown.png)
>  #expand  skin/classic/calendar/calendar-toolbar.css   
> (/calendar/base/themes/__THEME__/calendar-toolbar.css)
>  #expand  skin/classic/calendar/calendar-views.css     
> (/calendar/base/themes/__THEME__/calendar-views.css)
> 
> This looks wrong.  Why does lightning need Sunbird's calenadr.css.  It's been
> getting buy without it so far.

calendar.css is referenced by /calendar/resources/content/calendarCreation.xul
 which is packaged with Lightning. We've been missing it up to this point, and I'm assuming Sunbird's is more up to date.

> 
> -#ifndef MOZ_SUNBIRD
> -    skin/classic/calendar/calendar.css (skin/classic/calendar.css)
> -    skin/classic/calendar/prevnextarrow.png (skin/classic/prevnextarrow.png)
> -    skin/classic/calendar/datetimepickers/minimonth.css
> (skin/classic/datetimepickers/minimonth.css)
> -#endif
> +% skin calendar classic/1.0 %skin/classic/calendar/
> The + line looks beyond the scope of this bug (removing detritus), can you
> explain what it's doing here?
Moving the manifest line from the top to next to the skin packaging lines for clarity

> +    skin/classic/calendar/unifinder/priority_low.png
> (skin/classic/unifinder/priority_low.png)
> +    skin/classic/calendar/week-view-corner.png
> (skin/classic/week-view-corner.png)
> 
> Same here.  What's needed about week-view-corner that wasn't needed before?
It was referred to when I lxred my list of files to delete. Truthfully it ISN'T needed and should be deleted, along with this line. It's referenced in sunbird/themes/*stripe/sunbird/calendar-views.css but I was distracted today when I was trying to find if the #weekview-daynumber-spacer-mid selector was still in use. I'll get back to you on that one. For now, assume I'll delete that added line and that .png gets added to the deletion list.

The unifinder-priority looks to be a "got rid of trailing whitespace" thing.
Comment on attachment 229583 [details] [diff] [review]
rev0 - stops packaging unused files 

-#expand  skin/classic/calendar/calendar-views.css	(/calendar/sunbird/themes/__THEME__/sunbird/calendar-views.css)
+#expand  skin/classic/calendar/calendar.css      (/calendar/sunbird/themes/__THEME__/sunbird/calendar.css)

The bug here seems to be that we include calendar.css in calendarCreation.xul, not that we don't package calendar.css.  Either way, this is huge overhead to burden lightning with, and we should find another solution if there really are rules that it cares about.

+    skin/classic/calendar/week-view-corner.png (skin/classic/week-view-corner.png)
Yeah, lose this.

r=jminta if we can just crop the calendar.css include from calendarCreation.xul, otherwise i'd like to see another iteration.
Attachment #229583 - Flags: first-review?(jminta) → first-review+
(In reply to comment #4)
> The bug here seems to be that we include calendar.css in calendarCreation.xul,
> not that we don't package calendar.css.  Either way, this is huge overhead to
> burden lightning with, and we should find another solution if there really are
> rules that it cares about.
There aren't. Line removed.

> > +    skin/classic/calendar/week-view-corner.png
> (skin/classic/week-view-corner.png)
> Yeah, lose this.
I also removed the unused selector #weekview-daynumber-spacer-mid from /m/cal/sunbird/themes/*stripe/sunbird/calendar-views.css which was the only place that .png was referenced from.

checked in on MOZILLA_1_8_BRANCH and trunk

leaving bug open to clean up remaining issues outside of /mozilla/calendar
Attachment #229841 - Flags: first-review? → first-review?(benjamin)
Attachment #229841 - Flags: first-review?(benjamin) → first-review+
(In reply to comment #6)
> Created an attachment (id=229841) [edit]
> patch b - rev0 - removes old xpi stuff from suite, and fixes allmakefiles.sh
> 

Patch b checked in on trunk. Baking before requesting a181
(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=229841) [edit]
> > patch b - rev0 - removes old xpi stuff from suite, and fixes allmakefiles.sh
> > 
> 
> Patch b checked in on trunk. Baking before requesting a181
> 
I think you missed removing calendar-window from xpfe/bootstrap/Makefile.in - it may be that ICONS section isn't used now (which is why it hasn't busted anything), but thats a separate bug ;-)

Also, seeing as you removed the inclusion of S80calendar_fix_permissions_bug_230617 from xpfe/bootstrap/init.d/Makefile.in surely that file can be removed now as there are no other references to it:

http://lxr.mozilla.org/seamonkey/search?string=S80calendar_fix_permissions_b
(In reply to comment #8)
> > Patch b checked in on trunk. Baking before requesting a181
> > 
> I think you missed removing calendar-window from xpfe/bootstrap/Makefile.in -

Sorry, forgot to mention, the icons set up in xpfe/bootstrap/Makefile.in will definitely be used because the copy of icons to suite/ hasn't been done on the 1.8.1 branch.
(In reply to comment #8)
> I think you missed removing calendar-window from xpfe/bootstrap/Makefile.in -
> it may be that ICONS section isn't used now (which is why it hasn't busted
> anything), but thats a separate bug ;-)
> 
> Also, seeing as you removed the inclusion of
> S80calendar_fix_permissions_bug_230617 from xpfe/bootstrap/init.d/Makefile.in
> surely that file can be removed now as there are no other references to it:

Correct on both counts. Patch c attached to address these.
Attachment #230589 - Flags: first-review?(benjamin)
Status: NEW → ASSIGNED
Attachment #230589 - Flags: first-review?(benjamin) → first-review+
patch c checked in on branch and trunk

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: