Closed Bug 1586743 Opened 5 months ago Closed 4 months ago

On trunk, the context menu on the subject no longer shows "Check Spelling" and "Languages"

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: jorgk-bmo, Assigned: aleca)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Looks like a fallout from the textbox to html:input conversion.

Flags: needinfo?(alessandro)

I'll take a look, thanks for reporting it.

Flags: needinfo?(alessandro)
Assignee: nobody → alessandro
Status: NEW → ASSIGNED

I can't seem to figure out why the spellchecker is not working.
The attribute should be correct, and since it seems that this is the only section where we're using it between us and m-c, I can't find a reference point to troubleshoot it.

Do you know if we can ping someone from m-c and see if they stumbled upon the same issue, or maybe missed it as well?

I'm kinda lost, sorry.

Meanwhile, I'm uploading this itty bitty patch that removes a double import of the globalOverlay.js file.

Attachment #9099365 - Flags: review?(jorgk)
Comment on attachment 9099365 [details] [diff] [review]
1586566-js-import.patch

OK.
Attachment #9099365 - Flags: review?(jorgk) → review+
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/dcd12e30d6d8
remove second import of globalOverlay.js from messengercompose.xul. r=jorgk

Some comments: You had the wrong bug number in the patch :/

As for the context menu: It's only the "standard" undo/cut/copy/paste/delete/select all menu that you get for free when you include the correct scripts from M-C.

What you want to do it this. Get a copy of FF Nightly. In about:config, the search box has a context menu with spell checking. Also, about:preferences has a search box with spell checking. You just need to see how that is done and copy the trick for the subject in the compose window.

Disclaimer: I tried the context menus in about:config and about:preferences in FF 68, not Nightly. So check Nightly first before you go hunting for the code that does it.

Maybe Tim can help us out here.
Sorry to bother you, but I can't figure out why the spellcheck attribute is not recognized and not adding the "Check Spelling" and "Languages" items in the context menu.

Flags: needinfo?(ntim.bugs)

(In reply to Alessandro Castellani (:aleca) from comment #7)

Maybe Tim can help us out here.
Sorry to bother you, but I can't figure out why the spellcheck attribute is not recognized and not adding the "Check Spelling" and "Languages" items in the context menu.

There are 2 different types of context menus for html <input>:

  • The ones used by pages loaded in the content area (eg. about:config, about:preferences), those are implemented and provided by nsContextMenu.js
  • The ones used by "top-level" windows (eg. messenger.xul) which editMenuOverlay.js provides

The spellcheck context menu Jorg mentions in comment 6 falls into the first category. Since editMenuOverlay.js doesn't implement the spellcheck menu, I assume that's why this has regressed here.

The easiest way to fix this would be to wrap the input inside a <moz-input-box spellcheck="true"> element (which is how the <textbox> binding implements spellcheck), we do plan to remove this however once the Firefox urlbar stops using <moz-input-box> (bug 1586591), although this is a custom element you could relatively easily port over to comm-central.

The other possible way would be forking editMenuOverlay.js to comm-central to add support for the spellcheck attribute.

Flags: needinfo?(ntim.bugs)

Thank you so much Tim for the detailed overview.
Magnus, I'm handing the decision over to you.
Which approach should we follow based on comment 8?

Flags: needinfo?(mkmelin+mozilla)

I don't think we want to fork editMenuOverlay.js, so I guess wrapping in <moz-input-box spellcheck="true"> is the easiest way.

Flags: needinfo?(mkmelin+mozilla)
Attached patch 1586743-spellcheck.patch (obsolete) — Splinter Review

This doesn't work as expected.

  • The context menu comes with all the items activated
  • The languages submenu doesn't work
  • When interacting with the input the console returns: JavaScript error: chrome://global/content/elements/textbox.js, line 93: TypeError: input is null

Any approximate timeline for when <moz-input-box> item will be removed?
I'd like to avoid working on a fix that won't last long.

Flags: needinfo?(ntim.bugs)
Flags: needinfo?(mkmelin+mozilla)

(In reply to Alessandro Castellani (:aleca) from comment #11)

  • When interacting with the input the console returns: JavaScript error: chrome://global/content/elements/textbox.js, line 93: TypeError: input is null

https://searchfox.org/mozilla-central/source/toolkit/content/widgets/textbox.js#224-229

Seems like the child input needs the .textbox-input class for this._input to be defined. Not sure if that fixes the two other issues, but you might want to try that first ?

Any approximate timeline for when <moz-input-box> item will be removed?
I'd like to avoid working on a fix that won't last long.

Whenever someone has time to work on bug 1586591, I'm not in a good position to know when this will be though.
But honestly, even if it gets removed from m-c, this is a custom element you should be able to port over to comm-central easily.

Flags: needinfo?(ntim.bugs)

The spellcheck works with the addition of the class, thanks Tim.
The languages submenu is not present, tho.

Magnus, should we directly create a CE for this matter in order to avoid relying on the textbox element?

Attachment #9100928 - Attachment is obsolete: true
Attachment #9100965 - Flags: feedback?(mkmelin+mozilla)
Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9100965 [details] [diff] [review]
1586743-spellcheck.patch

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

Yeah this works. I wouldn't do anything else at this point.
For the languages menu that works differently then e.g. in the editor. It shows the menu to choose language, but only if you have more than one dictionary installed. https://searchfox.org/mozilla-central/rev/97976753a21c1731e18177de9e5ce78ea3b3da2d/toolkit/content/widgets/textbox.js#163-166

That seems alright though. There are so many other places in the UI that will let you install a dictionary if you so desire.
Attachment #9100965 - Flags: feedback?(mkmelin+mozilla) → review+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/c4c283656a88
Enable spellcheck and languages item in subject line context menu. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Keywords: checkin-needed
Resolution: --- → FIXED

Thanks for fixing this just in time for TB 71 beta next week. I tested that switching the language in the subject also switches the language in the body and vice versa. Spelling with the various "editors" in the compose windows is a tricky thing.

Sadly I found bug 1589433 while I was testing :/

Target Milestone: --- → Thunderbird 71.0
You need to log in before you can comment on or make changes to this bug.