181312 - New calendar should initialize name from filename

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[gekacheka]

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

[Mike Potter]

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.

Comment 2

[Mike Potter]

This is fixed in CVS.

Comment 3

[gekacheka]

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

Comment 4

[Mike Potter]

New contact from [e-mail] to [e-mail]
Filter on string OttawaMBA to get rid of these messages. 
Sorry for the spam.

Comment 5

[charles]

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

[Victor Engmark]

*** Bug 273377 has been marked as a duplicate of this bug. ***

Comment 7

[Joey Minta]

Attachment: 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).

Comment 8

[gekacheka]

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 9

[Michiel van Leeuwen (email: mvl+moz@)]

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.

Comment 10

[gekacheka]

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

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

(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$");

Comment 12

[gekacheka]

Attachment: v2: init name from uri

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

updated jminta's patch as requested by mvl and me.

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

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

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in

Comment 15

[Simon Paquet [:sipaq]]

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

patch is obsolete

Comment 16

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!