344987 - Remove remaining xpi detritus from various places

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Matthew (lilmatt) Willis]

There's still some xpi gack remaining in various places that should be removed.

Comment 1

[Matthew (lilmatt) Willis]

Attachment: rev0 - stops packaging unused files

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

Comment 2

[Joey Minta]

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.

Comment 3

[Matthew (lilmatt) Willis]

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

[Joey Minta]

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.

Comment 5

[Matthew (lilmatt) Willis]

(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

Comment 6

[Matthew (lilmatt) Willis]

Attachment: patch b - rev0 - removes old xpi stuff from suite, and fixes allmakefiles.sh

Comment 7

[Matthew (lilmatt) Willis]

(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

Comment 8

[Mark Banner (:standard8)]

(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

Comment 9

[Mark Banner (:standard8)]

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

Comment 10

[Matthew (lilmatt) Willis]

Attachment: patch c - standard8's catches

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

Comment 11

[Matthew (lilmatt) Willis]

patch c checked in on branch and trunk

-> FIXED