330020 - [FIX]View navigation buttons does not work since 2006-03-08 build

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Stefan Sitter [:ssitter]]

View navigation buttons does not work since 2006-03-08 build

Steps to Reproduce:
1) Go to day/week/multiweek/month view.
2) Press one of the navigation buttons on top of the view to
   navigate view to next/previous day/week/month.

Actual Results:
Nothing happens. No error are shown in JavaScript console.

Expected Results:
View displays next/previous day/week/month.

Additional Information:
WORKS in Sunbird win32-2006-03-07-07-trunk build
FAILS in Sunbird win32-2006-03-08-07-trunk build
No changes to mozilla/calendar happened during that period.

Comment 1

[Stefan Sitter [:ssitter]]

Linux trunk builds are affected too.
1.8 branch build of Lightning 0.1 is *not* effected.

Comment 2

[Stefan Sitter [:ssitter]]

Checkins to directory mozilla between 2006-03-07 07:00 and 2006-03-08 07:00: http://tinyurl.com/q3vrh  Bug 234455 looks most suspicious.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Where is the source for these buttons in lxr?

Comment 4

[Stefan Sitter [:ssitter]]

(In reply to comment #3)
> Where is the source for these buttons in lxr?

Buttons are defined as xbl binding in http://lxr.mozilla.org/mozilla/source/calendar/base/content/calendar-decorated-base.xml#46 and are used in various xbl files http://lxr.mozilla.org/seamonkey/search?string=calendar-navigation-buttons
Maybe jminta can give more detailed information on this topic.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So are the oncommand event handlers triggered?  Or do we not get even that far?

Comment 6

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

Yes, the oncommand handlers are triggered and run correctly. The used variables looks fine, on a first look.
The move event listeners are never called though.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> The move event listeners are never called though.

When you create the "move" events you set them to not bubble.  Is that correct?  Note that XUL used to bubble all events no matter what, but that this got fixed.  See changes to colorpicker.xml and so forth in the patch for bug 234455.

Smaug, it looks like there are tons of remaining JS callers that pass false for canBubble.  Did you audit them while working on bug 234455 already?  Or does that need to happen?

Comment 8

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

(In reply to comment #7)
> Smaug, it looks like there are tons of remaining JS callers that pass false for
> canBubble.  Did you audit them while working on bug 234455 already?  Or does
> that need to happen?
> 
Well, I wouldn't say 'tons' ;) But I tried to look at as many as possible 
in FF, TH and SM, but unfortunately not in SB. So I didn't catch all of them
(not even in FF)
I have to admit that I'm a bit amazed how often the DOM Events API has been 
misused in XUL (though, not too many regressions yet).
But this was one of the reasons why I really wanted
to get the dispatching rewritten before 1.9a1. Now I have time to fix these
callers - changing .initEvent("eventName", false, true) to .initEvent("eventName", true, true)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Frankly, I think we should probably make that a blanket change...

Comment 10

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Attachment: proposed patch

Terribly sorry, I should have checked also Sunbird 
when testing bug 234455. I changed all of the initEvent calls I found
in calendar/ because I noticed that addEventListener is used usually for
bubble phase.

Is one r+ enough in calendar/ or r+r or r+sr?

Comment 11

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

(In reply to comment #10)
> because I noticed that addEventListener is used usually for
> bubble phase.

Er, I checked there is always at least one addEventListener("eventName", handler, false) case for each .initEvent, so the events really should bubble.

Comment 12

[Joey Minta]

Comment on attachment 214774 [details] [diff] [review]
proposed patch

Thanks for the patch! r=jminta.

(That's sufficient for checkin, but we're in a code-freeze right now for Lightning 0.1.)

Comment 13

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

(In reply to comment #12)
> (That's sufficient for checkin, but we're in a code-freeze right now for
> Lightning 0.1.)

So no checkins even in trunk?

Comment 14

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

(In reply to comment #13)
> So no checkins even in trunk?

For calendar, trunk and branch are syncronized (all checkins are done using cross-commit.py), so a frozen branch means frozen trunk.

Comment 15

[Stefan Sitter [:ssitter]]

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

Comment 16

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

I'll check this in asap calendar is not frozen anymore.

Comment 17

[Stefan Sitter [:ssitter]]

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

Comment 18

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Checked in to branch and trunk.

Comment 19

[Damian Szczepanik]

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

Comment 20

[Damian Szczepanik]

Litmus testcase 2499 created

Comment 21

[Damian Szczepanik]

Litmus testcase 2608 created