206382 - cannot enter start/end times for event using the keyboard, have to click

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, enhancement)

Description

[Juraj Ziegler]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030430 Debian/1.3-5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030430 Debian/1.3-5

When creating new events, the start/end date textfields can be entered through
the keyboard, but start/end times have to be entered by clicking. This is bad,
bad, bad. Typing them in through the keyboard would be way faster.

Reproducible: Always

Steps to Reproduce:




Expected Results:  
Allow me to fill the start/end time textfields through the keyboard.

Comment 1

[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 2

[Joachim Schlosser]

Duplicate of 203368

Comment 3

[gekacheka]

Attachment: Part 1 of 7: patch to eventDialog.xul

Changes to use new timepicker xbl component

Comment 4

[gekacheka]

Attachment: Part 2 of 7: patch to eventDialog.js

Changes to use new timepicker xbl component

Comment 5

[gekacheka]

Attachment: Part 3 of 7: timepicker/timepicker.css (new)

New file for xbl binding for timepicker.css

Comment 6

[gekacheka]

Attachment: Part 4 of 7: timepicker/timepicker.xml (new)

Timepicker xbl component (new)
Incorporates old timepicker-overlay.xul and timepicker.js

Comment 7

[gekacheka]

Attachment: Part 5 of 7: timepicker/timepicker-overlay.xul (remove)

Timepicker-overlay.xul is no longer needed, as it is incorporated into
timepicker.xml.  This patch replaces it with a comment saying what happened.

Comment 8

[gekacheka]

Attachment: Part 6 of 7: timepicker/timepicker.js (remove)

Timepicker.js is no longer needed, as it has been incorporated into
timepicker.xml.   This patch replaces its contents with a note saying what
happened.

Comment 9

[gekacheka]

Attachment: Part 7 of 7: patch to dateUtils.js

Add method to parse time, used by timepicker.

Comment 10

[David Fraser]

gekacheka, looks very interesting.
Unfortunately these patches were not done from CVS. Would it be possible for you
to get the latest version of the calendar code from CVS and create the
appropriate patches?
Also, it seems timepicker.xml is actually an .xul file, so it should propbable
be called timepicker.xul

Comment 11

[gekacheka]

Attachment: Part 1 of 7: patch to eventDialog.xul

Changes from recent cvs to use new timepicker (was changes from 20030916).

Comment 12

[gekacheka]

Attachment: Part 2 of 7: patch to eventDialog.js

Changes from recent cvs to use new timepicker (prev was changes from 20030916).

Timepicker behavior removed from eventDialog.js, collected in timepicker.xbl

Comment 13

[gekacheka]

Attachment: Part 3 of 7: timepicker/timepicker.css (new)

New XBL binding file for timepicker.
(previously patch pointed to timepicker.xml, now timepicker.xbl)

Comment 14

[gekacheka]

Attachment: Part 4 of 7: timepicker/timepicker.xbl (new)

New time picker XBL component, incorporates old timepicker-overlay.xul and old
timepicker.js, as well as some behavior from eventDialog.js.
(previous patch named file timepicker.xml [like datepicker.xml], now named
timepicker.xbl)

Comment 15

[gekacheka]

Attachment: Part 5 of 7: timepicker/timepicker-overlay.xul (remove)

Timepicker-overlay has been incorporated into timepicker.xbl, so this file is
no longer needed.  This patch replaces its contents with a comment about what
happened to it.  (Previous version of patch pointed to timepicker.xml, now
points to timepicker.xbl)

Comment 16

[gekacheka]

Attachment: Part 6 of 7: timepicker/timepicker.js (remove)

Timepicker.js has been incorporated into timepicker.xbl, so timepicker.js is no
longer needed.	This patch replaces its contents with a comment saying what
happened.  (previous version of patch pointed to timepicker.xml, now points to
timepicker.xbl)

Comment 17

[gekacheka]

Re comment #10:

Comparing against code from cvs changed the eventDialog.xul and eventDialog.js
patches.

Timepicker.xml is now named timepicker.xbl, which changed the 4 timepicker
patches.  (It was named timepicker.xml like datepicker.xml, should
datepicker.xml also be renamed?)

Comment 18

[David Fraser]

Can't give a final answer about the naming, maybe Mostafah can. Otherwise
patches work great... thanks

Comment 19

[gekacheka]

Thanks.  It turns out timepicker-overlay is referred to in the printDialog and
alertDialog, so these need to be changed as well.

Comment 20

[gekacheka]

I tried creating a datetimepicker.xbl which uses the datepicker and timepicker
and contains the code that sets and combines them together.  In the event dialog
which uses both the datetimepicker (for start/end) and the datepicker alone (for
recurrence end and exceptions), it works fine if all four are datetimepicker, or
all four are datepicker, but as designed with two datetimepickers and two
datepickers there seems to be an error in xbl during initialization, and it
breaks one of two ways.  I get errors like

Error: uncaught exception: [Exception... "Illegal operation on WrappedNative
prototype object"  nsresult: "0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)" 
location: "JS frame :: chrome://calendar/context/datetimepickers/datepicker.xbl
:: onxblconstructor :: line 114"  data: no]

which is in the constructor, and is breaking on a call to getAttribute on the
datepicker.  As I said above, it works if either datepicker or datetimepicker
are used alone, but perhaps because datetimepicker uses datepicker, when the
event dialog refers to both datetimepicker and datepicker somehow the
datepickers in the recurrence tab are not always getting initialized as objects
with attributes before the constructor runs.  Sometimes the recurrence
datepickers do work, but then the start/end datetimepickers do not work, so
there could be a timing error (race condition). 

Currently waiting for 1.6final to come out, maybe that will fix it.

Comment 21

[gekacheka]

Attachment: Zip of datetimepickers, eventDialog, dateUtils

Here is a zip of the files for eventDialog refactored and simplified by using a
datetimepicker.xbl that synchronizes a datepicker.xbl and timepicker.xbl
  dateUtils.js \
  datetimepickers/calendar.css \
  datetimepickers/calendar.xbl \
  datetimepickers/datepicker.css \
  datetimepickers/datepicker.xbl \
  datetimepickers/datetimepicker.css \
  datetimepickers/datetimepicker.xbl \
  datetimepickers/timepicker.css \
  datetimepickers/timepicker.xbl \
  eventDialog.xul \
  eventDialog.js

They work for me in the Sunbird 20040114 windows build.
(I unzipped the Sunbird/chrome/calendar.jar file, 
 renamed it to calendar.jar-disabled, and
 changed the calendar locations in chrome/installed-chrome.txt
 from 
    content,install,url,jar:resource:/chrome/calendar.jar!/content/calendar/
   
locale,install,url,jar:resource:/chrome/calendar.jar!/locale/en-US/calendar/
    skin,install,url,jar:resource:/chrome/calendar.jar!/skin/classic/calendar/
    skin,install,url,jar:resource:/chrome/calendar.jar!/skin/modern/calendar/
 to
    content,install,url,resource:/chrome/calendar/content/calendar/
    skin,install,url,resource:/chrome/calendar/skin/modern/calendar/
    skin,install,url,resource:/chrome/calendar/skin/classic/calendar/
    locale,install,url,resource:/chrome/calendar/locale/en-US/calendar/
 and installed the attached files to 
    Sunbird/chrome/calendar/content/calendar/ )

In Mozilla 1.6b or 1.6final, with 2004010914-cal it partially works.
(I installed the attached files to
    Mozilla/chrome/calendar/content/ )
The Event tab of the event dialog works fine, but the datepickers in the
Recurrence tab still cause errors ("Illegal operation on WrappedNative
prototype object") as described in comment #20, so they do not initialize
correctly and do not disable correctly.

This appears to be an underlying XBL problem (sometimes constructor is run
before it can access attributes).

Can anyone diagnose or work around this problem?

Comment 22

[Mostafa Hosseini]

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

Comment 23

[gekacheka]

Attachment: part 1 of 4: patch to dateUtils.js ( checked in )

Updated patch to dateUtils.js

Comment 24

[gekacheka]

Attachment: part 2 of 4: patch to eventDialog.xul

Updated patch to eventDialog.xul
(uses datetimepickers xbl components)

Comment 25

[gekacheka]

Attachment: part 3 of 4: patch to eventDialog.js

Updated patch to eventDialog.js
(simplified by using datetimepickers xbl components)

Comment 26

[gekacheka]

Attachment: part 4 of 4: zip of datetimepickers files

Zip of added files for datetimepickers xbl components (datepicker, timepicker,
datetimepicker) used by event dialog.

(The old datepicker and timepicker are still used by the todo dialog, but will
eventually be obsolete.  When they are eliminated, their content and skin
directories, and the ca-event-dialog skin directories, may also be eliminated.)

Comment 27

[Mostafa Hosseini]

Comment on attachment 145872 [details] [diff] [review]
part 1 of 4: patch to dateUtils.js ( checked in )

Checked in.

Comment 28

[Mostafa Hosseini]

Comment on attachment 145877 [details]
part 4 of 4: zip of datetimepickers files

Issues: 
1) There are "chrome://calendar/context/" entries in some of the files.
Shouldn't those be "content"? Some existing files in the calendar source are
using "context" too. I'm wondering if this is an error which has been falling
through the cracks until now.

2) Why not keep datepicker and timepicker  and enhance them as xbls to be
re-used wherever desired instead of making them obsolete? Maybe some other part
of Mozilla or even calendar in the future may need just a date picker or just a
time picker instead of the combo. The datetimepicker can still coexist as a
combo.

Comment 29

[gekacheka]

(In reply to comment #28)
> (From update of attachment 145877 [details])
> Issues: 
> 1) There are "chrome://calendar/context/" entries in some of the files.
> Shouldn't those be "content"? Some existing files in the calendar source are
> using "context" too. I'm wondering if this is an error which has been falling
> through the cracks until now.

Surprisingly it still works, but yes I think you may be right, as searching LXR
for "/context/" only turns up three old calendar xul files. 

In the attatchments for this bug, /context/ appears in eventDialog.xul, and in
instructive comments in three xbl files in datetimepickers, so can be changed to
"content" in all four places.

> 
> 2) Why not keep datepicker and timepicker  and enhance them as xbls to be
> re-used wherever desired 

Yes, this refactorization does make the datepicker and timepicker into
independent xbl components.  In fact, the datetimepicker is an xbl component
which uses both the datepicker xbl component and the timepicker xbl component. 
Also, the event dialog uses both the datepicker xbl component as well as the
datetimepicker component.  (Since the eventDialog.xul loads the datetimepicker
xbl component via css, which automatically loads the datepicker xbl component,
the eventDialog.xul doesn't need to explicitly load the latter, maybe that
confused you.  I'm not sure if xbl is smart enough to avoid the redundant work.)

The datepicker xbl component gets its triangle button images from skin rather
than content (so skins can change their color/appearance).  

It seemed unnecessary to have a bunch of separate directories each with only two
files in them, so all the date/time/datetime pickers are placed into one
directory.  Grouping them all together may also make clearer that any changes to
datepicker or timepicker may affect datetimepicker.

Comment 30

[Mostafa Hosseini]

Comment on attachment 145877 [details]
part 4 of 4: zip of datetimepickers files

Sounds good then.
Files checked in with minor modifications:
context->content
and some indenting fixes.

Another note: The change in attachment 136220 [details] [diff] [review] from bug 223505 has been undone
in the new calendar.xbl. We must make sure that bug is not  happening with the
new change.

Comment 31

[Justin Wood (:Callek)]

Mostafah, re-check that comit...

Bonsai showing 0/0 lines checked in with these files, (ver 1.1 of course)

and lxr reporting that the file does not exist...

didnt try a checkout yet.

"Checked in modified versions of attachment 145877 [details] for bug 206382"

Comment 32

[Justin Wood (:Callek)]

In-fact to clarify, your previous checkins to "datetimepicker" directory do not
exist either, seems cvs didnt create teh directory, lxr reporting "no such
directory" and no lines on those other files either, at the same time, bonsai
still says 0/0 lines on those "datetimepicker" files....

hope none of the other comits calls these, (again didnt try checkout)

Comment 33

[Justin Wood (:Callek)]

[sorry for spam]

2004-04-14 12:41  comit seems to check-out fine though bonsai and lxr seem to
hate it

though all the images sent to the directory are NOT fine, didnt checkout (using
mirror server), strange since they were comitted earlier

Comment 34

[gekacheka]

Mostafa, a checkin bug:  method name is clickDate, not openPopup.

mozilla/calendar/resources/content/datetimepickers/datepicker.xbl

@@ -188,7 +184,7 @@
             ]]></body>
         </method>
 
-        <method name="openPopup">
+      <method name="clickDate">
             <parameter name="aMiniMonthGrid" />
             <body><![CDATA[
                 this.update(new Date(aMiniMonthGrid.value), true)

Comment 35

[gekacheka]

(In reply to comment #30) 
> Another note: The change in attachment 136220 [details] [diff] [review] from bug 223505 has been undone
> in the new calendar.xbl. We must make sure that bug is not  happening with the
> new change.

The code submitted above did not change the minimonth sidebar, which still used
the old datepicker/calendar.xml

To make minimonth sidebar use the minimonth.xbl component need fixes in 
  comment #34
  bug 240816 (which depends on fixes in bug 240808, bug 240815)

Comment 36

[Mostafa Hosseini]

(In reply to comment #34)
> Mostafa, a checkin bug:  method name is clickDate, not openPopup.
> 

Fixed. Sorry.

Comment 37

[Brian Ghidinelli]

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

Comment 38

[Mostafa Hosseini]

Comment on attachment 145873 [details] [diff] [review]
part 2 of 4: patch to eventDialog.xul

Applying this attachment and the next one for eventDialog.js, I can't change
the time either by editting the value or by using the time picker. Is this
probably just my build? Is it working elsewhere using the latest calendar
trunk?

Comment 39

[Justin Wood (:Callek)]

Mostafah, the time-change "takes" in the dialog, though it does not take when I
exit the dialog, the calendar view leaves the old start/end time in there. 
Didnt check the iCal file/object(s) or what it is stored as in JS.

OS, WinXP Pro, cygwin install, MSVC7, latest nightly

Comment 40

[gekacheka]

Attachment: zip of patched eventDialog.xul, eventDialog.js

I tried patching against trunk, and it worked for me.  The problems updating
the display after closing the event dialog occur even without the patch, so
they must be due to other changes.

Attachment contains my version of files, in case you want to compare and see
where they might differ from your patched files. 

I added one more strictness fix, removing an extra 'var' in eventDialog.js:

@@ -1172,7 +1170,7 @@
    {
       //get the date from the date and time box.
       //returns a date object
-      var dateToAdd = document.getElementById( "exceptions-date-picker"
).value;
+      dateToAdd = document.getElementById( "exceptions-date-picker" ).value;
    }
    
    if( isAlreadyException( dateToAdd ) )

Comment 41

[Justin Wood (:Callek)]

I just tried on my build, minus those two patches to be sure, it is definately
something in them, WFM (without your patches, that is the time on view changing)

Comment 42

[Mostafa Hosseini]

The problem was:

http://bugzilla.mozilla.org/show_bug.cgi?id=240813#c3

The patches now work. I'll check them in.

Comment 43

[Mostafa Hosseini]

Comment on attachment 145873 [details] [diff] [review]
part 2 of 4: patch to eventDialog.xul

Checked in.

Comment 44

[Mostafa Hosseini]

Comment on attachment 145875 [details] [diff] [review]
part 3 of 4: patch to eventDialog.js

Checked in.
Thanks for the great improvement.

Comment 45

[Mostafa Hosseini]

Fixed in CVS.

Comment 46

[Jussi Kukkonen]

This is really great. Two issues though:

1. Most of the files in content/datetimepickers/ don't have a license block.

2. I'm missing the datepicker-buttons and timepicker-buttons! I can click the
area in event dialog where the buttons should be and everything works, but the
buttons themselves don't appear. I'm using a build from 2004-05-04 with classic
skin. This is pretty strange and could be another bug of course...

Comment 47

[gekacheka]

Icon problem might be bug 240233

Comment 48

[Reed Loden [:reed]]

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