Closed Bug 1532595 Opened 6 years ago Closed 6 years ago

Port Bug 1513343 Remove textarea binding and replace usages with html:textarea

Categories

(Thunderbird :: General, task)

Product:
Thunderbird
Component:
General
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 67.0

People

(Reporter: jorgk-bmo, Assigned: mkmelin)

References

Details

Attachments

(3 files)

bug1532595_textarea_dexbl.patch
(deleted), patch
Paenglab
: review+
Details | Diff | Splinter Review
bug1532595_textarea_dexbl.patch
(deleted), patch
mkmelin
: review+
Details | Diff | Splinter Review
1532595-textareas-1.diff
(deleted), patch
Paenglab
: review+
Details | Diff | Splinter Review

12:50:24 - Paenglab_work: Bug 1513343 is on autoland and will probably break us. We have a lot of multiline textboxes.
12:52:46 - ntim: Paenglab_work: normally, you should only need to change all usages to textareas and import the context menu scripts if not already done
12:53:53 - ntim: Paenglab_work: if you're using multiline textboxes with spellcheck support then the spellcheck will likely no longer work
12:54:07 - ntim: the spellcheck context menu*

Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Blocks: tb-war-on-xbl
No longer blocks: 1513343
Depends on: 1513343
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Attached patch bug1532595_textarea_dexbl.patch (deleted) — — Splinter Review

This should be it.
Ran out of time, back later in the evening.

The chat changes may be slightly off. Can't test properly since chat is broken atm.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9048475 - Flags: review?(richard.marti)
Comment on attachment 9048475 [details] [diff] [review] bug1532595_textarea_dexbl.patch Review of attachment 9048475 [details] [diff] [review]: ----------------------------------------------------------------- I give a r+ but the calendar textboxes need some work as they don't flex correctly. The other places look good. ::: calendar/base/content/calendar-task-view.xul @@ +228,5 @@ > </row> > </rows> > </grid> > </hbox> > + <html:textarea id="calendar-task-details-description" flex="1"/> This needs some work as the textbox isn't tall enough. ::: calendar/base/content/dialogs/calendar-summary-dialog.xul @@ +357,5 @@ > control="item-description" class="header"/> > <separator class="groove" flex="1"/> > </hbox> > <box orient="horizontal" flex="1"> > + <html:textarea id="item-description" rows="6" flex="1"/> Also this textbox isn't wide enough.
Attachment #9048475 - Flags: review?(richard.marti) → review+
Attached patch bug1532595_textarea_dexbl.patch (deleted) — — Splinter Review

Some slight update, fixed a textbox.inputField reference

The things you mention are still a bit broken.

Attachment #9048652 - Flags: review+

Leaving this open for follow-ups and test fixes, if any.

Keywords: leave-open
Target Milestone: --- → Thunderbird 67.0
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/994cd1a152ed Port bug 1513343: Replace usages of textarea binding with html:textarea. r=Paenglab
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/f7f19b653be0 fix linting white-space issues. rs=white-space-only DONTBUILD

Test failures here in Calendar. Can you get to them before I disable them?

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(geoff)
Pushed by geoff@darktrojan.net: https://hg.mozilla.org/comm-central/rev/5a8945615c42 Fix broken calendar event dialog tests; rs=bustage-fix

Okay, I fixed the tests, but the event dialog is still wrong visually. I can't get the textbox to flex both horizontally and vertically.

Flags: needinfo?(geoff)

Thanks, but the tree is still not happy. testTaskView.js is a perma-fail now (bug 1522605?) and on debug runs we see the disconserting:

Assertion failure: IsInAnonymousSubtree(), at z:/build/build/src/dom/base/Element.cpp:1869
PROCESS-CRASH | testTodayPane.js | application crashed [@ mozilla::dom::Element::UnbindFromTree(bool,bool)]

I'll file a bug for that.

Pushed by geoff@darktrojan.net: https://hg.mozilla.org/comm-central/rev/9f7cb4ba09a9 Fix broken calendar task view test; rs=bustage-fix

Are we done here? No, still more work required, see comment #4.

Attached patch 1532595-textareas-1.diff (deleted) — — Splinter Review

I think this should fix all the remaining issues.

Attachment #9051180 - Flags: review?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9051180 [details] [diff] [review] 1532595-textareas-1.diff Thanks, that fixes it. I see a little jumping of the textboxes between the three tabs in event. But this isn't caused by you.
Attachment #9051180 - Flags: review?(richard.marti) → review+
Keywords: leave-open → checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/11f46ea9ea72
Fix display of textareas in calendar. r=Paenglab

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Regressions: 1556203

How would I port a binding like this:

<binding id="inputzombie" extends="chrome://global/content/platformHTMLBindings.xml#inputFields">

it leads to bad coordinates (negative height) in chrome input elements in Tb68b5.

That binding was removed - see bug 1419091. I don't think you can port it directly, but you need to change approach.

Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: