1527535 - [de-xbl] Replace address book datepicker widget with non-XBL elements

Status: RESOLVED FIXED

(Thunderbird :: Address Book, task)

Description

[Geoff Lankow (:darktrojan)]

Like bug 1524456, but for the date picker in the address book, which doesn't need to be a date picker at all. I'm replacing it with two menulists.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: 1527535-addrbook-datepicker-1.diff

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9043494 [details] [diff] [review]
1527535-addrbook-datepicker-1.diff

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

Yes, works. I think if we don't have a month, the day should also reset.

::: mail/components/addrbook/content/abCard.inc.xul
@@ +229,5 @@
>            <spacer flex="1"/>
>            <label control="Birthday" value="&Birthday.label;"
>                   accesskey="&Birthday.accesskey;"/>
>            <hbox class="AddressCardEditWidth" align="center">
> +            <menulist id="BirthMonth">

any reason not just to simply use <html:select> for this?

@@ +239,5 @@
> +              <menupopup>
> +                <menuitem label="" value="-1"/>
> +              </menupopup>
> +            </menulist>
> +            <textbox id="BirthYear" maxlength="4" class="YearWidth" />

please keep the placeholder. Actually, the other menus also would benfit from a description for the empty value.

@@ +241,5 @@
> +              </menupopup>
> +            </menulist>
> +            <textbox id="BirthYear" maxlength="4" class="YearWidth" />
> +            <label control="Age" value="&Age.label;"/>
> +            <textbox id="Age" maxlength="4" class="YearWidth" />

and <html:input type="number">

::: mail/components/addrbook/content/abCard.js
@@ +405,5 @@
> +      birthDay.menupopup.appendChild(menuitem);
> +    }
> +  }
> +
> +  birthMonth.addEventListener("command", setDisabledMonthDays);

I think it would nicer to pass in birthMonth and birthDate to setDisabledMonthDays so we don't need to know the ids two times

Comment 3

[Geoff Lankow (:darktrojan)]

(In reply to Magnus Melin [:mkmelin] from comment #2)

•        <menulist id="BirthMonth">
  

any reason not just to simply use <html:select> for this?

Yes. It's a XUL dialog, nothing else in it uses an HTML control, and they look awful. Plus the stylesheet doesn't want to work properly on HTML namespace stuff, I tried adding the namespace to the stylesheet, all sorts of things looked wrong afterwards.

and <html:input type="number">

Doesn't offer us anything other than some spinbuttons. You can still enter garbage in there. At this point I'm just wasting time trying to get it to look right.

please keep the placeholder. Actually, the other menus also would benfit
from a description for the empty value.

Okay.

I think it would nicer to pass in birthMonth and birthDate to
setDisabledMonthDays so we don't need to know the ids two times

And instead have to fudge the addEventListener calls? This is much tidier IMO.

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: 1527535-addrbook-datepicker-2.diff

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9044538 [details] [diff] [review]
1527535-addrbook-datepicker-2.diff

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

Looks good, except for that leap year handling has some bug. The UI still allows (+sticks) to Feb 29 2001 etc. In the display it changes over to March 1.
r=mkmelin with that fixed

Comment 6

[Geoff Lankow (:darktrojan)]

Attachment: 1527535-addrbook-datepicker-3.diff

This can't be landed yet because it needs bug 1524456 to land first.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/395833021007
Remove datepicker widget from address book; r=mkmelin

Comment 8

[Frank-Rainer Grahl (:frg)]

I am having no problem with removing common bindings but this the second time now that it broke SeaMonkey compile without warning. I or IanN would really like to have an advance information.

I know SeaMonkey is broken but we still keep it compiling for a possible later fixup.

Thanks
FRG

Comment 9

[Frank-Rainer Grahl (:frg)]

Attachment: Add old datepicker.xml to suite

bustage fix for suite

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ebe97898c429
Copy common datepicker.xml to suite bindings. rs=bustage-fix

Comment 11

[Jorg K (CEST = GMT+2)]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/395833021007
Remove datepicker widget from address book; r=mkmelin

I came here since I noticed that the date picker in the address book doesn't work in TB 66 beta, the month is stuck at January. Now, the patch rips out datetimepicker.xml altogether, which won't make certain add-ons very happy.

Comment 12

[Jorg K (CEST = GMT+2)]

Sigh, I've already reported that as bug 1492560.

Comment 13

[Magnus Melin [:mkmelin]]

There's really very little reason for someone to use it. <input type="date"> gives you all you need. Well, if it would. Bug 1527615.