348245 - Datepicker not working anymore after using date/year dropdown

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

Datepicker not working anymore after using date/year dropdown.
Probably caused by checkin for Bug 346318.

Steps to Reproduce:
1. Open New Event dialog
2. Open the datepicker for start date
3. Click the month name or year on top to open the dropdown menu
4. Select another month or year in the dropdown menu
5. Repeat step 3. and 4.
6. Select specific day in minimonth shown below

Actual Results:
Nothing happens. No dropdown menu is shown. 
The clicked day is not selected. Datepicker does not close.

Expected Results:
Should work as in nightly from last day.

Works in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/2006080914 Calendar/0.3a2+

Fails in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/2006081006 Calendar/0.3a2+ and Lightning/0.1+ (2006081006).

Same with linux build.

Comment 1

[Matthew (lilmatt) Willis]

Confirmed on Mac as well
HW -> all

Comment 2

[Stefan Sitter [:ssitter]]

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

Comment 3

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

The patch for bug 346318 is just all wrong. It just removes the code that was supposed to prevent this bug.
I think the best way to go would be to back out that patch, and rethink it. Then write a patch that doesn't remove the eventlisteners etc.

Comment 4

[Michael Büttner (no reviews TFN)]

I just started to investiage this issue and found that the current behavior is not just annoying but can also lead to a fatal crash. Open the dialog, click on the drop-down arrow of the datepicker, click on the month-slection, press escape twice. This leads to a crash, at least on windows. tested with the latest version on trunk. I'll continue working on this bug.

Comment 5

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

First of all this patch reverts most of the stuff from bug 346318 and prevents the fatal crash. since i currently can't test this on a mac, i don't know whether or not this patch reintroduces the problem bug 346318 tried to solve. i would suspect that this is probably indeed the case. but as mvl already suggested, i would revert the patch, reopen bug 346318 and go for a better solution.

Comment 6

[Stefan Sitter [:ssitter]]

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

Comment 7

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

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

r=mvl
This does break mac, but at least makes linux not crash.

Comment 8

[Michael Büttner (no reviews TFN)]

Attachment: patch v2

i just tried to verify the fix before checking in the patch and found that performing an arbitrary selection in the month/year popups leads to a crash. i tested this on windows with the current 1.8 branch version. i'm sure that i tested this when i originally commited this patch, but i don't know why this behavior didn't show up previously, probably i was sleep-walking...

the cause for the crash is the call to hidePopup() in the click handler of the minimonth binding. switching the month/year triggers the reshowPopup() method in the datetimepicker binding, which in turn calls hidePopupList() in the minimonth, which tries to call hidePopup() again because it has in internal reference to the popup. the fix is to call hidePopupList() instead of hidePopup() to get rid of the internal reference.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

After we check in this fix, we're still going to need to address the Mac problem, ideally by having mento/josh help us un-regress the trunk, right?

Comment 10

[Michael Büttner (no reviews TFN)]

(In reply to comment #9)
> After we check in this fix, we're still going to need to address the Mac
> problem, ideally by having mento/josh help us un-regress the trunk, right?
I didn't try this on Mac, but I would suspect that it still has problems after this patch has been checked in. In light of this, yes, the above statement is correct.

Comment 11

[Michael Büttner (no reviews TFN)]

Comment on attachment 238173 [details] [diff] [review]
patch v2

thomas is sick, he won't be in town for the next couple of days.

Comment 12

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

Before checking this in, i'd like to get someone with a mac to spend some time with bug 346318. (But if no mac owner volunteers, we have no choice but to break the datapicker on mac. That would be unfortunate...)

Comment 13

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

Comment on attachment 238173 [details] [diff] [review]
patch v2

r=mvl, be see previous before check-in.

Comment 14

[Matthew (lilmatt) Willis]

Comment on attachment 238173 [details] [diff] [review]
patch v2

r=lilmatt

Let's take this and work on fixing whatever broke the Mac separately with mento and josh.

Comment 15

[Matthew (lilmatt) Willis]

Patch updated on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Yes, we know this breaks the datepicker on the Mac.
Please see bug 346318.

Comment 16

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3

Comment 17

[Damian Szczepanik]

Litmus testcase 2642 created

Comment 18

[Omar Bajraszewski]

Maybe should it be reopened because of bug 356505?