Closed Bug 1562998 Opened 1 year ago Closed 5 months ago

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

Categories

(Thunderbird :: General, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 70.0

People

(Reporter: aleca, Assigned: aleca)

References

Details

Attachments

(1 file, 3 obsolete files)

Assignee: nobody → alessandro
Mentor: alessandro
Status: NEW → ASSIGNED
Depends on: 1562997
Attached patch 1562998-textbox-html-input.patch (obsolete) — Splinter Review

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.

Attachment #9084378 - Flags: review?(mkmelin+mozilla)
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...
Attachment #9084378 - Flags: review?(mkmelin+mozilla) → feedback+

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

Attached patch 1562998-textbox-html-input.patch (obsolete) — Splinter Review

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.

Attachment #9084378 - Attachment is obsolete: true
Attachment #9085131 - Flags: review?(mkmelin+mozilla)
Attached patch 1562998-textbox-html-input.patch (obsolete) — Splinter Review
Attachment #9085131 - Attachment is obsolete: true
Attachment #9085131 - Flags: review?(mkmelin+mozilla)
Attachment #9085216 - Flags: review?(mkmelin+mozilla)

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

Flags: needinfo?(jorgk)

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.

Flags: needinfo?(jorgk)

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:

&lt;script src="chrome://global/content/globalOverlay.js"/>
&lt;script src="chrome://global/content/editMenuOverlay.js"/>

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

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
Attachment #9085216 - Flags: review?(mkmelin+mozilla) → feedback+

Great, updated!

Attachment #9085216 - Attachment is obsolete: true
Attachment #9085497 - Flags: review?(mkmelin+mozilla)
Attachment #9085497 - Flags: feedback+
Attachment #9085497 - Flags: review?(mkmelin+mozilla) → review?(paul)
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.
Attachment #9085497 - Flags: review?(paul) → review+

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

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0
You need to log in before you can comment on or make changes to this bug.