Closed
Bug 458368
Opened 16 years ago
Closed 16 years ago
Move and integrate calendar/prototypes to calendar/base
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b1
People
(Reporter: mschroeder, Assigned: mschroeder)
Details
Attachments
(2 files, 4 obsolete files)
203.46 KB,
patch
|
dbo
:
review+
Fallen
:
review+
|
Details | Diff | Splinter Review |
221.56 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → mschroeder
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•16 years ago
|
||
Just the code changes. Merging, renaming, and moving of files still missing.
Assignee | ||
Comment 2•16 years ago
|
||
This patch contains all code changes (also the merge of sun-calendar-event-dialog.dtd and calendar-event-dialog.dtd), but no (real) renames and moves.
Attachment #341700 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #341840 -
Attachment is patch: true
Attachment #341840 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 3•16 years ago
|
||
Attachment #341840 -
Attachment is obsolete: true
Attachment #342782 -
Flags: review?(daniel.boelzle)
Comment 4•16 years ago
|
||
Comment on attachment 342782 [details] [diff] [review] Patch v1 >diff --git a/calendar/prototypes/themes/pinstripe/timezone_0h.png b/calendar/base/themes/pinstripe/pinstripe/timezone_0h.png >rename from calendar/prototypes/themes/pinstripe/timezone_0h.png >rename to calendar/base/themes/pinstripe/pinstripe/timezone_0h.png I can't test the patch (the jar script complains about missing files when building): The timezone chooser files have been moved into .../pinstripe/pinstripe/... r- for now
Attachment #342782 -
Flags: review?(daniel.boelzle) → review-
Assignee | ||
Comment 5•16 years ago
|
||
Attachment #342782 -
Attachment is obsolete: true
Attachment #342803 -
Flags: review?(daniel.boelzle)
Comment 6•16 years ago
|
||
It's good to move all the prototypes stuff away in one move but I think we should create a base/content/dialogs/ and and respectively a base/themes/pinstripe|winstriype/dialogs folder for all our dialogs. calendar/base /content has become a container for nearly everything. While you are at it you could then also move the recurrence dialog files to that new destination. I also find that the naming of our files could be shortened: If a dialog is located in calendar/base/content/... it is redundant to have a "calendar-" substring in the filename.
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > It's good to move all the prototypes stuff away in one move but I think we > should create a base/content/dialogs/ and and respectively a > base/themes/pinstripe|winstripe/dialogs folder for all our dialogs. IMO this is something we should handle in a second bug or patch.
Comment 8•16 years ago
|
||
Comment on attachment 342803 [details] [diff] [review] Patch v2 >diff --git a/calendar/locales/en-US/chrome/calendar/calendar-event-dialog.dtd b/calendar/locales/en-US/chrome/calendar/calendar-event-dialog.dtd >--- a/calendar/locales/en-US/chrome/calendar/calendar-event-dialog.dtd >+++ b/calendar/locales/en-US/chrome/calendar/calendar-event-dialog.dtd >@@ -1,46 +1,54 @@ > > [...] > >- - The Original Code is OEone Calendar Code, released October 31st, 2001. >+ - The Original Code is Sun Microsystems code. > - >- - The Initial Developer of the Original Code is >- - OEone Corporation. >- - Portions created by the Initial Developer are Copyright (C) 2001 >+ - The Initial Developer of the Original Code is Sun Microsystems. >+ - Portions created by the Initial Developer are Copyright (C) 2006 > - the Initial Developer. All Rights Reserved. > - > - Contributor(s): >- - Cédric Corazza <cedric.corazza@wanadoo.fr> >+ - Michael Buettner <michael.buettner@sun.com> >+ - Philipp Kewisch <mozilla@kewis.ch> >+ - Stefan Sitter <ssitter@gmail.com> >+ - Martin Schroeder <mschroeder@mozilla.x-home.org> >+ - Hubert Gajewski <hubert@hubertgajewski.com>, Aviary.pl >+ - Fred Jendrzejewski <fred.jen@web.de> >+ - Cedric Corazza <cedric.corazza@wanadoo.fr> While it is correct that you bring over the contributors to sun-calendar-event-dialog.dtd, you should not change the "Original Code" and "Initial Developer" sections. This file was originally created by OEone developers and that still remains the case. Please remove the Sun Microsystems attributions. >diff --git a/calendar/locales/jar.mn b/calendar/locales/jar.mn >--- a/calendar/locales/jar.mn >+++ b/calendar/locales/jar.mn >@@ -18,12 +18,12 @@ calendar-@AB_CD@.jar: > > [...] > >- locale/@AB_CD@/calendar/sun-calendar-event-dialog.dtd (%chrome/prototypes/sun-calendar-event-dialog.dtd) >- locale/@AB_CD@/calendar/sun-calendar-event-dialog.properties (%chrome/prototypes/sun-calendar-event-dialog.properties) >- locale/@AB_CD@/calendar/calendar-invitations-dialog.dtd (%chrome/prototypes/calendar-invitations-dialog.dtd) >- locale/@AB_CD@/calendar/calendar-subscriptions-dialog.dtd (%chrome/calendar/calendar-subscriptions-dialog.dtd) >+ locale/@AB_CD@/calendar/calendar-event-dialog.dtd (%chrome/calendar/calendar-event-dialog.dtd) >+ locale/@AB_CD@/calendar/calendar-event-dialog.properties (%chrome/calendar/calendar-event-dialog.properties) >+ locale/@AB_CD@/calendar/calendar-invitations-dialog.dtd (%chrome/calendar/calendar-invitations-dialog.dtd) >+ locale/@AB_CD@/calendar/calendar-subscriptions-dialog.dtd (%chrome/calendar/calendar-subscriptions-dialog.dtd) We already include calendar-event-dialog.dtd in line 8 of this file. So there's no need to include it a second time.
Comment 9•16 years ago
|
||
Oh, and IMO it would be great if you could move resources/content/mouseoverPreviews.js to base/content/ as well, while you are touching it.
Comment 10•16 years ago
|
||
Personally I'd like to see files in resources/ stay there until they are overworked. There are a couple of things that could be improved, w.r.t style and coding constructs. In the case of mouseoverPreviews.js, this file is going away in bug 366680 as soon as I have time to resurrect those patches.
Comment 11•16 years ago
|
||
(In reply to comment #7) > (In reply to comment #6) > > It's good to move all the prototypes stuff away in one move but I think we > > should create a base/content/dialogs/ and and respectively a > > base/themes/pinstripe|winstripe/dialogs folder for all our dialogs. > > IMO this is something we should handle in a second bug or patch. Since base/content is already quite crowded, I second Berend's proposal. We should move the prototype dialogs to their final destination in this bug, all others be left out resp. subject to follow-up bugs, of course.
Assignee | ||
Comment 12•16 years ago
|
||
* incorporated Simon's comments * created dialog directories in base/themes/*instripe & base/content
Attachment #342803 -
Attachment is obsolete: true
Attachment #342907 -
Flags: review?(daniel.boelzle)
Attachment #342803 -
Flags: review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #342907 -
Flags: review?(philipp)
Attachment #342907 -
Flags: review?(daniel.boelzle)
Attachment #342907 -
Flags: review+
Comment 13•16 years ago
|
||
Comment on attachment 342907 [details] [diff] [review] Patch v3 You should put calendar-invitations-manager.js into base/content/ instead of base/content/dialogs/. Apart from that the patch looks good and works for me. r=dbo with that fixed Requesting 2nd review from Philipp.
Comment 14•16 years ago
|
||
Comment on attachment 342907 [details] [diff] [review] Patch v3 Looks good, r=philipp I didn't have a chance to do real testing, but from looking at the applied patch all is good. I would appreciate if you could go ahead and align the jar files you are changing to have the "(" at column 60 and make the file use #filter substitution and @THEME@, since my sunbird.jar patch will hopelessly bitrot when this is checked in.
Attachment #342907 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 15•16 years ago
|
||
Addressed Daniel's and Philipp's comments.
Assignee | ||
Comment 16•16 years ago
|
||
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/62876a69cb35> -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Updated•14 years ago
|
Target Milestone: 1.0 → 1.0b1
You need to log in
before you can comment on or make changes to this bug.
Description
•