1562998 - Use HTML input instead of XUL textbox in calendar-unifinder-todo.xul

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Alessandro Castellani [:aleca]]

https://searchfox.org/comm-central/rev/94d2c771f935e9402ca3d19a43fafeeaa8f0ac4f/calendar/base/content/calendar-unifinder-todo.xul#41

Comment 1

[Alessandro Castellani [:aleca]]

Attachment: 1562998-textbox-html-input.patch

To fully test this, you'd need to load the patch on bug 1562997 if it hasn't landed yet, as some JS and CSS are shared.

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9084378 [details] [diff] [review]
1562998-textbox-html-input.patch

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

LGTM

::: calendar/base/content/calendar-unifinder-todo.xul
@@ +21,5 @@
>    <vbox id="todo-tab-panel" persist="height,collapsed" flex="1">
>      <!-- This second vbox means all of this is added to the DOM at once,
>           so the label's binding doesn't complain about not having a control. -->
>      <vbox flex="1">
>        <box id="todo-label" align="left" collapsed="true">

hmm, looks to me this box (and label) are never shown? not due to this patch...

Comment 3

[Alessandro Castellani [:aleca]]

Indeed, that box and label are always collapsed.
Are we safe to remove it?

Comment 4

[Alessandro Castellani [:aleca]]

Attachment: 1562998-textbox-html-input.patch

I removed that unused label and launched a try run to see if any tests fail.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=676d0d48257e44b0a9d0c26cd368dafafad73c90

Accessibility wise, that label wasn't adding anything useful to the tasks tree pane.

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: 1562998-textbox-html-input.patch

A couple of test failures fixed.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3a6fea228767bc77a326c9e5bd16f51bd930502f

Comment 6

[Alessandro Castellani [:aleca]]

Mhh, this is odd.
Those 2 Mochitest failures pass locally after the updated patch.
Any idea how I can troubleshoot this?

Comment 7

[Jorg K (CEST = GMT+2)]

Mochitest, not my territory. How do you run them, I'd like to learn. In return I can tell you that these fail all the time, so don't worry.

Comment 8

[Alessandro Castellani [:aleca]]

For this particular test I run ./mach mochitest mail/components/extensions/test/browser/browser_ext_browserAction.js from the source/ directory.

It was failing locally the first time, but I fixed it after adding the scripts necessary for an html:input to enable the right click context menu:

<script src="chrome://global/content/globalOverlay.js"/>
<script src="chrome://global/content/editMenuOverlay.js"/>

But as you saw it keeps failing with the try-run.

Comment 9

[Jorg K (CEST = GMT+2)]

Thanks, noted :-)

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9085216 [details] [diff] [review]
1562998-textbox-html-input.patch

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

Looks good

::: calendar/base/content/calendar-unifinder-todo.xul
@@ -21,5 @@
>      <!-- This second vbox means all of this is added to the DOM at once,
>           so the label's binding doesn't complain about not having a control. -->
>      <vbox flex="1">
> -      <box id="todo-label" align="left" collapsed="true">
> -        <label flex="1" crop="end" style="font-weight: bold" value="&calendar.unifinder.todoitems.label;" control="unifinder-todo-tree"/>

should remove the calendar.unifinder.todoitems.label from the dtd too

Comment 11

[Alessandro Castellani [:aleca]]

Attachment: 1562998-textbox-html-input.patch

Great, updated!

Comment 12

[Paul Morris [:pmorris]]

Comment on attachment 9085497 [details] [diff] [review]
1562998-textbox-html-input.patch

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

r+ Looks good and works as expected.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/91539be83495
textbox to html input in calendar-unifinder-todo.xul. r=mkmelin,pmorris