1414521 - Port bug 1412361 to calendar: Remove checkbox-baseline and checkbox-radio xbl bindings, fix failing test testTodayPane.js

Status: RESOLVED FIXED

(Calendar :: General, enhancement, P3)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #1412361 +++

Used here: https://dxr.mozilla.org/comm-central/source/calendar/base/content/widgets/calendar-widgets.xml#256

Something is probably broken.

Comment 1

[Richard Marti (:Paenglab)]

And this too: Bug 1411640 - Consolidate <radio> bindings and styling across platforms.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: checkbox-baseline.patch

This fixes the in toolkit removed binding like done at https://hg.mozilla.org/mozilla-central/rev/31b3ba0e8491#l4.12

But I get the error:

TypeError: aPeriod.listItem.getCheckbox(...).setChecked is not a function[Learn More]  agenda-listbox.js:92:5
	agendaListbox.addPeriodListItem chrome://calendar/content/agenda-listbox.js:92:5
	agendaListbox.init chrome://calendar/content/agenda-listbox.js:35:5
	agendaListbox.setupCalendar chrome://calendar/content/agenda-listbox.js:686:5
	onLoad chrome://calendar/content/today-pane.js:33:9

When I remove everything in this function, more elements are shown in the today pane, but I get this errors:

ReferenceError: reference to undefined property "listItem"[Learn More]  agenda-listbox.js:727:9
TypeError: curPeriod.listItem is undefined[Learn More] agenda-listbox.js:727:9
	agendaListbox.refreshPeriodDates chrome://calendar/content/agenda-listbox.js:727:9
	updatePeriod chrome://calendar/content/today-pane.js:395:9
	setDay chrome://calendar/content/today-pane.js:367:9
	setDaywithjsDate chrome://calendar/content/today-pane.js:337:9
	onchange chrome://messenger/content/messenger.xul:1:1
	fireEvent chrome://calendar/content/widgets/minimonth.xml:971:13
	update chrome://calendar/content/widgets/minimonth.xml:1104:17
	set_value chrome://calendar/content/widgets/minimonth.xml:407:1
	setDay chrome://calendar/content/today-pane.js:365:64
	initializeMiniday chrome://calendar/content/today-pane.js:117:9
	onLoad chrome://calendar/content/today-pane.js:34:9

Comment 3

[Jorg K (CEST = GMT+2)]

Hmm, is this the reason why
mozmake -C calendar/test/mozmill SOLO_TEST=testTodayPane.js mozmill-one
fails? See bug 1414533 comment #2.

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Richard Marti (:Paenglab) from comment #2)
> TypeError: aPeriod.listItem.getCheckbox(...).setChecked is not a
> function[Learn More]  agenda-listbox.js:92:5

Yes setChecked was implemented in checkbox-baseline

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 8925261 [details] [diff] [review]
checkbox-baseline.patch

Review of attachment 8925261 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't tested it but looks correct I think.

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #4)
> Yes setChecked was implemented in checkbox-baseline
Magnus is referring to this:
https://hg.mozilla.org/mozilla-central/rev/31b3ba0e8491#l1.52

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: checkbox-baseline.patch (v2 JK)

This passes the test. All ideas by Aceman via IRC.

Richard, does this look right?

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: checkbox-baseline.patch (v3)

These are really Aceman's ideas.

Comment 9

[Jorg K (CEST = GMT+2)]

This fixes the test. There are visual glitches, so I guess we need to fix some CSS or so.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b35420759ed0
Port 1412361 to calendar: Use checkbox instead of checkbox-baseline (all ideas by aceman). rs=bustage-fix

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8925306 [details] [diff] [review]
checkbox-baseline.patch (v3)

This is an incomplete patch which returns the functionality with a visual glitch. I hope Richard can look into that.

Comment 12

[Richard Marti (:Paenglab)]

You where too fast. I was on uploading the patch with CSS fix.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: checkbox-baseline-css.patch

This fixes the visual glitch.

Comment 14

[Jorg K (CEST = GMT+2)]

Comment on attachment 8925307 [details] [diff] [review]
checkbox-baseline-css.patch

Works for me, thanks. Why wasn't the "center no-repeat" needed before?

I wasn't too fast. We had 7 bustages today:
- two I fixed in the morning
- the ">" crashes (bug 1414513, bug 1414544)
- this bug here
- all the Mozmill failures (bug 1414533) caused by the
- prefs issue (bug 1414398)
You want me to be any slower??

Comment 15

[Jorg K (CEST = GMT+2)]

Sorry, make that 8: Win32 busted, bug 1414507. Sadly I seem to be involved in most of them in some way, so whatever I can close, I will close.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fe6af7970776
Port 1412361 to calendar: Use checkbox instead of checkbox-baseline (visual fixes). r=jorgk