278877 - Sunbird 0.2RC2 Freeze if I click on drop-down month in new/edit event

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Skeeter Allen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

When I create or edit a new event, if I drop down the calendar in the start or
end day and then click on the month (January) or the year (2005) rather than use
arrows, app freezes and the drop-down paints in front of any other window
(except Task Manager). Clicking on OK does nothing. Cancel or close crashes the app.

Reproducible: Always

Steps to Reproduce:
1.Open Sunbird
2. Click New Event
3. Click the down arrow on Start or End.
4. Click on the name of the month or year.

Actual Results:  
App freezes.
Drop-down paints on top of all apps
Cancel or close crashes app

Expected Results:  
Nothing? another drop-down of months/years?

Comment 1

[gekacheka]

Bug 199779 is where this problem arose, and has a possible but imperfect patch.

Confirming this bug as blocking bug 199779 instead of marking it duplicate
because it is easier to find.

Marking this bug target 0.2.

Comment 2

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

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

Comment 3

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

Attachment: patch v1

This patch uses popups in a popupset. That made most crashes go away. To get
rid of the few i could still cause, i had to manually make sure there is always
only one popup open.
All this feels like working around xul bugs, but i don't feel like poking the
guts of xul.

Comment 4

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

Comment on attachment 176171 [details] [diff] [review]
patch v1

>+            <!-- XXX Make this keyboard accessible -->
This comment shouldn't be here. Not because it is wrong, but because is messes
the dom, so childNode[n] doesn't work like it should. Just imagine the comment
is gone.

Comment 5

[gekacheka]

Comment on attachment 176171 [details] [diff] [review]
patch v1

This patch does not work for me, tried using Moz1.8b1 and TB1.0, both on w2k. 
After the month or year menu pops down, everything is frozen just as before, so
cannot select a date or change month or year.

Comment 6

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

Then maybe i have to make the <datepicker/> use popupset and popop too, not
menulist.
(but weird that the patch does prevent crashers on linux)

Comment 7

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

Attachment: also make datepicker use popupset

Another try. This patch also changes the datepicker to use a popupset/popup
instead of a menulist. I imitated the look of the menulist. This might be
improved a bit (the css needs to move to the theme anyway) but i want to see if
it works first.
It also fixes the redrawing issue for me (monthname not redrawn when selecting
a month with a shorter name)

Comment 8

[gekacheka]

Comment on attachment 176485 [details] [diff] [review]
also make datepicker use popupset

This still doesn't work for me on w2k.	It still freezes as before.  

It is a little different: before when it freezes, the only way to get it to
popdown was to click the popup [v] button.  Now clicking the [v] button pops
down and immediately pops it up again back at the initial date unfrozen.

(Maybe it would be easier to find a workaround for one of the testcases of bug
280373, then apply it to  datepicker/minimonth.   I haven't figured out any
workaround, but maybe someone else can.)

Comment 9

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

When exactly does it freeze? does it involve opening the month popup?

And this is an attempt to work around bug 280373, by trying a different
solution. And the solution somehow works for me on linux.

Comment 10

[gekacheka]

Yes it freezes as before:
  new event (event dialog appears)
  click start date popdown [v] (minimonth popup appears)
  click month name (popup list of month names appears)
  click a different month (popuplist drops down, minimonth changes to new month)
  click a date in month (frozen: popup stays up and does not change, dialog
title bar and dialog task bar button blink once, sort of like when a modal
dialog has focus but you click outside it.)
  click on year, month, backward month, or forward month buttons (still frozen:
popups do not popup, month displayed does not change, 
  click the start date popdown [v] (the minimonth popup disappears so the dialog
underneath may be visible for a fraction of a second, then the minimonth popup
appears again, but back at the initial date.  Or sometimes the dialog underneath
is never becomes visible [maybe timing of redraw is different] so it just looks
like the popup is redrawn back at the original date.  In either case the result
is the popup is showing, whereas before the result was the popup was hidden.)

  

Comment 11

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

From the description i conclude that sunbird doesn't crash, as it used to do. Or
did it only really crash on linux, and was windows only a temporary freeze?
Also, is the complete app frozen, or only the popup and maybe the dialog?

If this patch fixes the linux crashes, and the redraw issue, i think it is worth
finding out why the input is blocked. Maybe there should be some onhidingpopup
listeners be added, to do some magic. Also, closing the popup by clikcing the
dropdown can be fixed by a little bit smarter onclick handlers.

Comment 12

[Skeeter Allen]

(In reply to comment #11)
> From the description i conclude that sunbird doesn't crash, as it used to do. Or
> did it only really crash on linux, and was windows only a temporary freeze?
> Also, is the complete app frozen, or only the popup and maybe the dialog?
> 
> If this patch fixes the linux crashes, and the redraw issue, i think it is worth
> finding out why the input is blocked. Maybe there should be some onhidingpopup
> listeners be added, to do some magic. Also, closing the popup by clikcing the
> dropdown can be fixed by a little bit smarter onclick handlers.

On Win2K, it was a complete freeze. Everything Sunbird (besides the popup)
stopped responding and painting. (If you covered part of Sunbird with another
window, then removed the covering window, the covered part is now background
grey.) As for temporary vs. permanent, I waited 30sec, 5 minutes and 15 minutes
in three tests so I'd call it functionally permanent.

Comment 13

[gekacheka]

(In reply to comment #12)
> On Win2K, it was a complete freeze. Everything Sunbird (besides the popup)
> stopped responding and painting. (If you covered part of Sunbird with another
> window, then removed the covering window, the covered part is now background
> grey.) As for temporary vs. permanent, I waited 30sec, 5 minutes and 15 minutes
> in three tests so I'd call it functionally permanent.

Does this happen for you with Sunbird 0.2 final? 

For me on w2k(sp4), it still redraws if you cover the window with another window
and then uncover it.  It also highlights the dates in the popup minimonth as you
move the mouse over it.  It just doesn't do anything except blink if you click.

The freeze may seem permanent until you learn that clicking the popdown [v]
button can get you out of it. 


Regarding crashing: I now see that it crashes if you get out of the freeze by
clicking 'ok' or 'cancel' rather than clicking the [v] button.   And yes, with
either of mvl's patches, it only freezes the popup and doesn't crash when
clicking 'ok' or 'cancel', so either patch is an improvement.  

With the first patch (datepicker_3), when a month is selected from the month
list, the text date in the text field is erased (field has no text) and a
vertical bar cursor blinks.  If you click on a date in the popup, the original
date reappears in the text field (just as when it loses focus with an
unparseable date, it reverts to original date).   So some problems may be caused
by the text field getting focus when it should not.   (Also, still does not
redraw April month, bug 273914.)

With the second patch (datepicker_4), when a month is selected from the month
list, the text date in the text field stays selected and no blinking cursor
appears.  So text field is not getting focus, an improvement over first patch,
but somehow popup is still not receiving click.  (And it does redraw month April
as mvl mentioned in comment #7, addressing bug 273914.)  

Mvl, let me know if you want me to re-review the second patch, datepicker_4, and
if so, please explain the purpose of the onpopupshowing and onpopuphiding
methods (how to reproduce problem they fix).

Comment 14

[Skeeter Allen]

(In reply to comment #13)

> Does this happen for you with Sunbird 0.2 final? 
I just re-downloaded the final and re-installed. It looks like 0.2 final fixes
the behavior that I logged for w2k. I guess I downloaded final too close to it's
release (not checking the version in "about") and was still using 0.2RC2.

Comment 15

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

the onpopupshowing handler is to prevent te event from bubbling to the
datepicker, which would then reset the date on the minimonth popup. But it might
be that canceling the event might be what causes the freeze.
gekacheka, can you test if removing the handler fixes the freeze? It will be the
return of the auto-reset, but that can also be solved in other ways.

Comment 16

[gekacheka]

Removing the onpopupshowing and onpopuphiding handlers from month-list and
year-list popups seems to make no difference (it still freezes whether they are
present or removed), but I'm not sure what to look for.  Can you explain when
"reset the date on the minimonth popup" occurs? (how to reproduce)?

Comment 17

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

I mean the problem that if you select a different month in the month popup, and
then open the month popup again, it resets to the original month. (because the
datepicker has an onpopup handler that sets the minimonth to the correct date)

Comment 18

[gekacheka]

On w2k it currently can't popup a second list (because it is 'frozen' after the
first popup list is hidden), so it seems to make no difference if they are removed.

Comment 19

[Andrew Douglas]

Attachment: Fix freeze on month/year drop down click

This patch works by having minimonth force the datepicker to refresh the popup
when month/year is changed. There are still possible freeze scenarios on
windows that will require xul work or UI rewrite. But this gets the month/year
dropdowns functional inside the datepicker.
-Andrew

Comment 20

[gekacheka]

Comment on attachment 177172 [details] [diff] [review]
Fix freeze on month/year drop down click

Thanks, this is getting pretty close.  However, I don't think it needs to
remove the menulist and replace it with the dropmarker.  Also, this.mOpen is
never used.  I'll have an updated patch soon.

Comment 21

[gekacheka]

Attachment: datepicker/minimonth patch for SB0.2 branch: fix freeze, fix title update (bug 273914)

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

Fixes several related problems with datepicker minimonth popups:

* Problem: Date reinitilized when month popuplist or year popuplist appears in
  datepicker popup.

  Fix: As mvl found, this seems to be caused by the onpopupshowing
  event propagating from the child popup to the parent popup, causing
  the parent popup onPopup method to be called when the child popup
  appears, which reinitialized the date.  The solution is give the
  child popups onPopupshowing handlers which call
  event.preventBubble() so they won't propagate to the parent popup.

* Problem: crash or freeze when month popuplist or year popuplist
  is popped up from datepicker popup.

  Fixes: 
  As mvl found, using a popupset may help avoid the crash.
  As andrew found, hiding and reshowing the parent popup
    when the child popup is hidden prevents the freeze.
  As andrew found, a complication is that reshowing the
    parent patch calls its onPopup handler which
    reinitializes the state, so the reshow code
    sets a flag to tell the datepicker onPopup handler not to
    reinitialize the minimonth during a reshowing.
  An added improvement is onpopuphiding handlers on the child
    popups to reshow the parent when the user hides the child
    popup by clicking outside it, avoiding problem mentioned
    by andrew.

* Title month and year are not always redisplayed when the month
  changes (bug 273914).

  Fix:	As andrew found, always hiding and reshowing minimonth 
  popup whenever the month changes ensures that it is displayed
  correctly.
   
Also fixes a few minor nits:
* While debugging, I found that sometimes an operation on the
  end datepicker would cause the start datepicker to become
  selected.  This seemed to be caused by the use of ids find
  nodes, so ids were removed from internal nodes in the xbl content,
  and now navigation is used to find all internal nodes.
* fields that are initialized once and never changed are
  now named with an initial 'k' (konstant) rather than 'm'
  (mutable member).

Issues:
* Minor: Clicking on the month or year to popup a selection and then
  clicking on it again should popup down the child popup, but now it
  pops down the entire parent datepicker popup as well.  
  - month/year text onclick hander does not receive the event.
  - minimonth click handler does not receive the event.
* Minor: the child popups are made of xul:text rather than xul:menuitem.
  Using menuitem produced a bug that clears the value of the
  datepicker text field when a child popup value is selected.

Comment 22

[gekacheka]

Comment on attachment 177264 [details] [diff] [review]
datepicker/minimonth patch for SB0.2 branch:  fix freeze, fix title update (bug 273914)

Tested using SB0.2 branch calendar extension of Moz1.8b1 and TB1.0 on w2k.

Navigation Minimonth:
* forward and backward arrows advance month
* months advance through April, May, July, July (no title update problem)
* Year increases when advancing from December to January.
* Year decreases when advancing from January to December.
* month popup selects month with same year.
* year popup selects year with same month.

Event Dialog end-date Datepicker Minimonth:
* forward and backward arrows advance month
* months advance through April, May, July, July (no title update problem)
* Year increases when advancing from December to January.
* Year decreases when advancing from January to December.
* month popup selects month with same year.  Next:
  - selecting year pops up year list (not frozen)
  - selecting year does not revert month to original month.
* year popup selects year with same month.  Next:
  - selecting month pops up month list (not frozen)
  - selecting month does not revert year to original year.
* popping up months popup and then clicking outside popup on event
  dialog pops down months popup.  Ditto for year popup.
* selecting day of month pops down minimonth and updates text field.

Comment 23

[gekacheka]

Attachment: datepicker/minimonth patch for trunk: fix freeze, fix title update (bug 273914)

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

Version of above patch adapted for trunk.  See previous notes.	
(Can test against trunk nightly when one becomes available for w2k.)

I hope it works on other platforms than w2k.

Comment 24

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

Comment on attachment 177265 [details] [diff] [review]
datepicker/minimonth patch for trunk: fix freeze, fix title update (bug 273914)

this still crashes for me on linux :(

to crash, i open the datepicker, click on the month, then on the year. That
gives 2 popups. Then closing the dialog with the 'ok' button crashes. 
I think there should be a check to close one popup when opening another.

Also, why did you change the monthpopup from having menuitems to just text? I
think menuitems look better.

Comment 25

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

Attachment: limit to one popup

This patch limits to one popup, and that prevents the crash on my linux box. I
can only hope that it still doesn't freeze windows.

I didn't change the text->menulist, but the text really doesn't look nice. No
feedback on hover for example. But let's change one thing at a time.

Comment 26

[gekacheka]

Comment on attachment 177296 [details] [diff] [review]
limit to one popup

Looks ok, I was about to submit the same change.

Suggest adding a comment 

+	       if (this.mPopup)
+		   this.mPopup.hidePopup();  // avoid crash on linux

(I took it out as it was not needed on w2k and I was hoping some other change
rendered it unnecessary, but I understand it's still needed.)

Comment 27

[gekacheka]

(In reply to comment #24) 
> Also, why did you change the monthpopup from having menuitems to just text? I
> think menuitems look better.

Reason mentioned at the very end of comment #21.  Have you encountered this
field-clearing problem when you use menuitems, or is it a w2k-only thing?

Comment 28

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

I checked in the latest patch. THere might be more work needed, to make it look
better, but at least it won't freeze or crash now.

Comment 29

[prabahy]

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

Comment 30

[Reed Loden [:reed]]

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