Closed Bug 1074645 Opened 10 years ago Closed 10 years ago

ReferenceError: acceptDialog is not defined [TEST-UNEXPECTED-FAIL | testTodayPane.js | testTodayPane.js::testTodayPane] [TEST-UNEXPECTED-FAIL | testLocalICS.js | testLocalICS.js::testLocalICS]

Categories

(Calendar :: General, defect)

Product:
Calendar
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mkmelin, Assigned: Fallen)

References

Details

(Keywords: intermittent-failure, regression)

Apparently perma-orange on windows. https://tbpl.mozilla.org/php/getParsedLog.php?id=49167987&tree=Thunderbird-Trunk&full=1#error5 SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\mozmill\testTodayPane.js | testTodayPane.js::testTodayPane EXCEPTION: waitFor: Timeout exceeded for 'function () {return mozmill.utils.getWindows("Calendar:EventDialog").length == 0}' at: utils.js line 447 TimeoutError utils.js:447 13 waitFor utils.js:485 1 MozMillController.prototype.waitFor controller.js:685 3 testTodayPane testTodayPane.js:67 3 Runner.prototype.wrapper frame.js:585 9 Runner.prototype._runTestModule frame.js:655 9 Runner.prototype.runTestModule frame.js:701 3 Runner.prototype.runTestFile frame.js:534 3 runTestFile frame.js:713 3 Bridge.prototype._execFunction server.js:179 10 Bridge.prototype.execFunction server.js:183 16 Session.prototype.receive server.js:283 3 AsyncRead.prototype.onDataAvailable server.js:88 3 TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
Markus, Merike, can one of you take a look here?
From log: > JavaScript error: resource://gre/components/nsPhishingProtectionApplication.js, line 156: TypeError: listManager.setUpdateUrl is not a function Bug 1032767 > JavaScript error: chrome://calendar/content/calendar-task-editing.js, line 102: TypeError: calendar is null Probably Bug 735253 and can be ignored > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: acceptDialog is not defined This one seems new and is most probably cause of the test failure.
Yeah, the Save and Save&Exit functionality of the event dialog is broken. Works in Thunderbird 35.0a1 (20140922030203) Fails in Thunderbird 35.0a1 (20140930030203) Unfortunately there were no builds in between. Changes during regression range: https://hg.mozilla.org/comm-central/pushloghtml?fromchange=9717f3be5a20&tochange=d9ef713a866e https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5bd6e09f074e&tochange=7c24470b6b3a
Keywords: regression
I managed to reduce regression range to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=435732392989&tochange=1735ff2bb23e This error seems to be caused by Bug 1030420. If I set "dom.compartment_per_addon" back to false the event dialog starts working again and the error is gone. I'm curious why the problem shows up only on Windows. Does dom.compartment_per_addon works different on platforms? Or is it only enabled for Windows nightly builds? @bholley: This is problem with running Lightning calendaring extension in Thunderbird. Do you have an idea where to start looking?
Basically, compartment_per_addon hoists XUL overlays into a separate compartment whose global is a sandbox with the window as its prototype. This is a mostly-transparent trick, and our goal is for it not to cause significant addon breakage. If you can track down the source of the problem, we'll see whether it's any kind of general problem that we might need to handle to keep things smooth. There isn't any platform-specific behavior in this change, but it could trigger bugs in existing platform-specific code.
Summary: TEST-UNEXPECTED-FAIL | C:\slave\test\build\mozmill\testTodayPane.js | testTodayPane.js::testTodayPane → ReferenceError: acceptDialog is not defined [TEST-UNEXPECTED-FAIL | testTodayPane.js | testTodayPane.js::testTodayPane] [TEST-UNEXPECTED-FAIL | testLocalICS.js | testLocalICS.js::testLocalICS]
Depends on: 1030420
The dialog is defined here: https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-event-dialog.xul https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-event-dialog.js Inside the dialog it defines a command that calls acceptDialog() of the dialog and that is used in e.g. toolbar button: https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-event-dialog.xul#136 This works on Mac and Linux but fails on Windows when dom.compartment_per_addon is true. The only difference I saw are different features passed in when opening the dialog: https://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-item-editing.js#475 Could this be realted? Any other idea?
The time picker widget in the dialog is broken too and produces similar errors: > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: clickHour is not defined > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: clickMinute is not defined > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: clickMore is not defined > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: doubleClickHour is not defined > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: onHide is not defined > JavaScript error: chrome://calendar/content/calendar-event-dialog.xul, line 1: ReferenceError: onPopup is not defined
The click/hide/popup errors seem to be oncommand event handlers called in XBL. Until now it was possible to execute the function directly like this: <content> <xul:button oncommand="foo()"/> </content> <implementation> <method name="foo"> ... </method> </implementation> On the other hand the acceptDialog error has nothing to do with XBL. Bobby, when you are back from PTO, could you let us know if the way we are using XBL just luckily succeeded or if this is fallout from the feature itself?
Flags: needinfo?(bobbyholley)
I would like to note that the acceptDialog does not work for me neither in Windows7 64bit, nor in Kubuntu 32bit - as Stefan mentioned that it would work on Linux... not for me.
I assumed it works on the other systems because the test failure is only reported on the Windows builders. Could someone test if the Save&Close button or the time picker widget works on Mac or not?
(In reply to Stefan Sitter from comment #14) > I assumed it works on the other systems because the test failure is only > reported on the Windows builders. No. Please search "acceptDialog" in the log. https://tbpl.mozilla.org/php/getParsedLog.php?id=49665107&tree=Thunderbird-Trunk&full=1
Fails on other platforms too per previous comment. Is it a known problem that failing tests are not reported to tbpl.mozilla.org for those platforms?
OS: Windows 7 → All
Hardware: x86_64 → All
I *think* it just that a few lines down, on linux you get command timed out: 7200 seconds without output running ... which somehow prevents it getting "properly" reported. mac was busted, but showing boatloads of calendar now: https://tbpl.mozilla.org/php/getParsedLog.php?id=49722812&tree=Thunderbird-Trunk&full=1
We have found a solution for the acceptDialog and timepicker issues, it also requires looking at all the other xbl binding calls. I will take care of this after the summit. I hope this fixes all the other issues.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Flags: needinfo?(bobbyholley)
(In reply to Philipp Kewisch [:Fallen] from comment #20) > We have found a solution for the acceptDialog and timepicker issues, it also > requires looking at all the other xbl binding calls. I will take care of > this after the summit. > > I hope this fixes all the other issues. Sorry for the delayed response - I've been trying to think of what might be going wrong. Can you elaborate on what you found? If the same problem is likely to bite other addons, we may need a platform-side fix for this.
Flags: needinfo?(philipp)
Sure thing. The code we wrote made use of some shortcuts and likely passed review because it "just worked". There were two cases. The first case [1], the acceptDialog case, we for some reason assumed that calling a function in the oncommand handler that is part of the documentElement magically appears on the window itself. This had been working before. The fix is simply to call "document.documentElement.acceptDialog()" instead of "acceptDialog()". In the second case its a matter of scoping between two xbl bindings. We have a binding for a timepicker, which has content that uses another binding. The inner binding has an onclick handler in its content [2], that simply calls "clickHour(...)". clickHour is not part of the inner binding, but instead part of the outer binding [3]. For some reason this worked before, I haven't actually fixed this but I assume its a matter of either propagating the click event out into the parent binding, firing a custom event, or using document.getBindingParent(this.parentNode). [1] http://hg.mozilla.org/comm-central/file/3cf5411b9643/calendar/base/content/dialogs/calendar-event-dialog.xul#l140 [2] http://hg.mozilla.org/comm-central/file/61dd7dc61dc4/calendar/resources/content/datetimepickers/datetimepickers.xml#l691 [3] http://hg.mozilla.org/comm-central/file/61dd7dc61dc4/calendar/resources/content/datetimepickers/datetimepickers.xml#l1188
Flags: needinfo?(philipp)
(In reply to Philipp Kewisch [:Fallen] from comment #23) > The first case [1], the acceptDialog case, we for some reason assumed that > calling a function in the oncommand handler that is part of the > documentElement magically appears on the window itself. This had been > working before. The fix is simply to call > "document.documentElement.acceptDialog()" instead of "acceptDialog()". As yes, this is bug 1083648. > In the second case its a matter of scoping between two xbl bindings. We have > a binding for a timepicker, which has content that uses another binding. The > inner binding has an onclick handler in its content [2], that simply calls > "clickHour(...)". clickHour is not part of the inner binding, but instead > part of the outer binding [3]. For some reason this worked before, I haven't > actually fixed this but I assume its a matter of either propagating the > click event out into the parent binding, firing a custom event, or using > document.getBindingParent(this.parentNode). I'm not positive, think this is actually the same issue as the above - we're depending on accessing the outer XBL API by virtue of the fact that the anonymous content being parented to the outer bound element. This is pretty clear evidence that we need to fix bug 1083648 for addon-compat.
Blocks: 1030420
Depends on: 1083648
No longer depends on: 1030420
(In reply to Bobby Holley (:bholley) from comment #24) > (In reply to Philipp Kewisch [:Fallen] from comment #23) > > In the second case its a matter of scoping between two xbl bindings. We have > > a binding for a timepicker, which has content that uses another binding. The > > inner binding has an onclick handler in its content [2], that simply calls > > "clickHour(...)". clickHour is not part of the inner binding, but instead > > part of the outer binding [3]. For some reason this worked before, I haven't > > actually fixed this but I assume its a matter of either propagating the > > click event out into the parent binding, firing a custom event, or using > > document.getBindingParent(this.parentNode). > > I'm not positive, think this is actually the same issue as the above - we're > depending on accessing the outer XBL API by virtue of the fact that the > anonymous content being parented to the outer bound element. So looking at our code, there are two cases. One would be a binding accessing its own methods from its <content> using onclick="call_a_method_on_this_binding()". The other case would be calling a method on the parent using onclick="call_a_method_on_parent_binding()". The question I have is what is left to do after you fix bug 1083648? Will this bug magically fix one or both mentioned cases, meaning I just have to wait until its done? Or should I be fixing the second case and expecting the first one to be solved? Is there maybe another method that isn't as long to call a method on this binding? My current workaround adds document.getBindingParent(this) before each method call, but it really makes the lines longer to read.
Flags: needinfo?(bobbyholley)
(In reply to Philipp Kewisch [:Fallen] from comment #25) > So looking at our code, there are two cases. One would be a binding > accessing its own methods from its <content> What do you mean by this? If Anonymous Content in <content> has an onclick handler, the |this| argument would be the Anonymous Content, not the bound content (its parent) with the <implementation>, right? > using > onclick="call_a_method_on_this_binding()". The other case would be calling a > method on the parent using onclick="call_a_method_on_parent_binding()". > > The question I have is what is left to do after you fix bug 1083648? Will > this bug magically fix one or both mentioned cases, meaning I just have to > wait until its done? Or should I be fixing the second case and expecting the > first one to be solved? I _think_ that both of them should be solved. > Is there maybe another method that isn't as long to call a method on this > binding? My current workaround adds document.getBindingParent(this) before > each method call, but it really makes the lines longer to read. Yeah :-(
Flags: needinfo?(bobbyholley)
(In reply to Bobby Holley (:bholley) from comment #26) > (In reply to Philipp Kewisch [:Fallen] from comment #25) > > So looking at our code, there are two cases. One would be a binding > > accessing its own methods from its <content> > > What do you mean by this? If Anonymous Content in <content> has an onclick > handler, the |this| argument would be the Anonymous Content, not the bound > content (its parent) with the <implementation>, right? Yes, using onclick="this.method()" will reference the anonymous content, but using onclick="method()" used to reference the parent binding, having the same effect as onclick="document.getBindingParent(this).method()". The second mentioned case would be onclick="method()" magically calling method on the parent binding, essentially being the same as onclick="document.getBindingParent(document.getBindingParent(this).parentNode)).method()" > > > using > > onclick="call_a_method_on_this_binding()". The other case would be calling a > > method on the parent using onclick="call_a_method_on_parent_binding()". > > > > The question I have is what is left to do after you fix bug 1083648? Will > > this bug magically fix one or both mentioned cases, meaning I just have to > > wait until its done? Or should I be fixing the second case and expecting the > > first one to be solved? > > I _think_ that both of them should be solved. Ok, then I'd like to wait for that bug fixed before I make any major changes here. I hope its high priority, as this is breaking our tests :-)
It's high priority. Patches are up in bug 1083648; are you able to give them a shot?
Flags: needinfo?(philipp)
Woo-hoo, tests are no longer failing on Linux and Windows builders. On MacOSX some tests are still failing but report a different error: testLocalICS.js and testTodayPane.js fail with: > Test Failure: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXPCComponents_Utils.import] testBasicFunctionality.js fails with: > Test Failure: : could not find element ID: calendar-tab-button > Test Failure: defaultView.getCalendarManager is not a function Should we resolve this bug as Fixed and file new ones for the MacOSX failures? Or do they already exist? OT: would be nice if the test case names would contain a hint to Lightning/calendar. "testBasicFunctionality" is pretty generic.
The OS X issue is bug 1083374.
OK, resolving as fixed because the remaining xpcshell and mozill failures on Mac OS X are tracked in bug 1083374.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.8
Sorry for not getting back to you on this, was on a conference and things got delayed. But the push shows that it works! Thanks for everything.
Flags: needinfo?(philipp)
You need to log in before you can comment on or make changes to this bug.