New calendar should initialize name from filename

RESOLVED FIXED

Status

Calendar
Sunbird Only
--
minor
RESOLVED FIXED
16 years ago
12 years ago

People

(Reporter: gekacheka, Assigned: Joey Minta)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.28 KB, patch
Michiel van Leeuwen (email: mvl+moz@)
: first-review+
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b) Gecko/20021016
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b) Gecko/20021016, 2002111516-cal (downloaded 1121, but build id still says 1115)

If I select a calendar file (e.g., browse local file), and do not give a title,
the calendar is listed with a blank title.  The name should default to the file
name in this case.

Reproducible: Always

Steps to Reproduce:
1. New calendar
2. Browse for local ics file, e.g., /share/GroupMeetings.ics
3. Ok (leave title blank)

Actual Results:  
Calendar appears with blank title

Expected Results:  
Calendar appears with title GroupMeetings

This regular expression may be useful, unless there is a better way to take
apart pathnames and urls: 

var fullPathRegex = new RegExp(".*[/\\\\:]([^/\\\\:]+)[\.]ics$");

var path = "file:///c:\\my-directory\\My File.ics";

var filename;
if (fullPathRegex.test(path))
 filename = path.replace(fullPathRegex, "$1");

produces filename == "My File";

(bugs: only handles case where there is at least one directory separator and
there is a .ics suffix.)

Comment 1

16 years ago
Good idea.  I have implemented the reverse of this, which is to give it a file
name if none is specified. I never thought of this situation.
Status: NEW → ASSIGNED

Comment 2

15 years ago
This is fixed in CVS.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 3

15 years ago
Reopen:  This fix also needs to be applied to the "Tools | Subscribe to remote
calendar" command, where it is still needed (as of 20031416-cal).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 4

15 years ago
New contact from mikep@oeone.com to mostafah@oeone.com
Filter on string OttawaMBA to get rid of these messages. 
Sorry for the spam.
Assignee: mikep → mostafah
Status: REOPENED → NEW

Comment 5

14 years ago
Another correlary to this issue is that when importing an ical file by clicking
it's link, or by using the tools>"subscribe to remote calendar" and giving the
location of an ical file, the saved filename (in the users profile data) is
"CalendarDataFile1.ics" followed by "CalendarDataFile2.ics", etc.  It would be
nice if these filenames were copied from the source file.

Comment 6

13 years ago
*** Bug 273377 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

13 years ago
Created attachment 192426 [details] [diff] [review]
auto-complete name

This adds the desired functionality to the wizard.  Since local calendar
locations aren't chosen anymore, this only applies to remote calendars. 
However, whenever possible it completes the name.  It uses gekacheka's
suggested regexp.  It will not overwrite a user-entered name, if it exists
(such as when going back then forward again).
Assignee: mostafah → jminta
Status: NEW → ASSIGNED
Attachment #192426 - Flags: first-review?(mvl)
(Reporter)

Comment 8

13 years ago
Comment on attachment 192426 [details] [diff] [review]
auto-complete name

suggestions:

1. I expect a function 'makeName' to return a name.  Maybe 'initNameFromURI'
would be a better function name.

2. If this is only for URLs, then there is no need to include the backslashes
in the regex (they were for DOS file paths).

3. What is the try/catch for?  It already checks that path is not null.

4. maybe use
  var nameField = document.getElementById("calendar-name");
Comment on attachment 192426 [details] [diff] [review]
auto-complete name

>+    var fullPathRegex = new RegExp(".*[/\\\\:]([^/\\\\:]+)[\.]ics$");

Why have two (escaped) backslashed in the charchter group? One would be enough,
i would think.

>+    try {
>+        var calName = path.replace(fullPathRegex, "$1");

Why not use match, and then use the result from that? using replace isn't the
common way to match something. It will also keep the regex shorter.
(Reporter)

Comment 10

13 years ago
(In reply to comment #9) 
> Why have two (escaped) backslashed in the charchter group? One would be enough,
> i would think.

Think again...  One set of escapes is for the string literal, reducing from four
to two in the string passed to RegExp.  Backslash is also the escape character
in regexps (such as in \s or \d), so it needs to be escaped in the character
group, reducing from two to one in the actual interpretation.  (Please don't be
confused by my extra string literal backslash in [\.], it is ineffective, it
might as well be [.]). 

> Why not use match, and then use the result from that? using replace isn't the
> common way to match something. It will also keep the regex shorter.

Trading off need to name the regex vs. need to construct and name the match
array.  (I assume you meant that it could shorten the expressions using the
regex by not naming it, not that it shortens the regex itself.)  Using the match
would be something like the following.

var filename;
// var matchArray = path.match(fullPathRegex); 
// same as next line.  prefer exec which has more complete documentation.
var matchArray = fullPathRegex.exec(path);
if (matchArray != null) 
  fileName = match[1];


(In reply to comment #10)
> Think again...
Bah. Too much use of perl...
> (I assume you meant that it could shorten the expressions using the
> regex by not naming it, not that it shortens the regex itself.)

No, i really mean that the expression itself can be shorter. You can leave out
everything before the part in ():

var fullPathRegex = new RegExp("([^/\\\\:]+)\.ics$");
(Reporter)

Comment 12

13 years ago
Created attachment 195588 [details] [diff] [review]
v2: init name from uri

(patch -l -p 2 -i file.patch)

updated jminta's patch as requested by mvl and me.
Attachment #192426 - Attachment is obsolete: true
Attachment #195588 - Flags: first-review?(mvl)
Comment on attachment 195588 [details] [diff] [review]
v2: init name from uri

Please keep the indentation consisten with the rest of the file. I know you
don't like 4 spaces, but we went for it, so we have to stick with it.
r=mvl with that changed
Attachment #195588 - Flags: first-review?(mvl) → first-review+
patch checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago13 years ago
Resolution: --- → FIXED
Comment on attachment 192426 [details] [diff] [review]
auto-complete name

patch is obsolete
Attachment #192426 - Flags: first-review?(mvl)
The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!
QA Contact: colint → sunbird
You need to log in before you can comment on or make changes to this bug.