1790388 - Double-click to select a word in composition can trigger the `Advanced Property Editor` dialog for <o:p> when replying to messages created with Microsoft Office

Status: NEW

(Thunderbird :: Message Compose Window, defect, P5)

Description

[Thomas D. (:thomas8)]

Attachment: Screenshot 1: Twitter user gets `Advanced Property Editor` dialog for <o:p> after double-clicking to select a word in reply

Jason got this as a complaint on Twitter (see screenshot below).

We don't know how often this happens in the wild, but when it does, it's unexpected and annyoing. Magnus once fixed a very similar issue in Bug 525124 - Double Clicking in a Blank Space Opens the Advanced Property Editor. And we've fixed many such issues before (bugs listed in see also).

New STR :

• see testcase 1 by thomas8 in comment 6, attachment 9294414 [details]
• Reply to a message created with Microsoft Office having empty <o:p></o:p> tag in a nested context (see original source in comment 4):
  <span>double-click HERE<o:p></o:p></span>
  You can create such message via Insert > HTML in composition, or use testcase 1 (attachment 9294414 [details]).
• In the quoted text, double-click on the last word ("HERE") before the <o:p> tag to select it for copying

Actual

• Advanced Property Editor dialog for <o:p> unexpectedly pops up and gets in the way
• Annoying whenever it happens.

Expected

• no dialog should pop up
• select the word only

---

I am failing to reproduce this with the above steps.
Here's the only a variant STR which reproduce for me on TB 102.2.2 (64-bit), Win10:

1. Compose new message
2. Insert > HTML dialog: Enter this code:
   <p><o:p>test</o:p></p>
   Note: <o:p> is a tag inserted by MS Office, apparently for Office Namespace [1]
3. Fast quadruple-click at the very left edge of the word test (just on the edge of or just outside the selection)

Actual

• Advanced Property Editor dialog pops up

Expected

• I don't think we'd ever want Advanced Property Editor dialog to pop up from just clicking about when selecting text.

[1] https://stackoverflow.com/questions/7808968/what-do-op-elements-do-anyway

Comment 1

[Thomas D. (:thomas8)]

Attachment: Screenshot 2: `Advanced Property Editor` dialog (en-US) for <o:p> after trying to double-click on a word (in reply)

Here's the original dialog in en-US .

Comment 2

[Thomas D. (:thomas8)]

https://searchfox.org/comm-central/search?path=&q=advanced%20property%20editor

Comment 3

[Alfred Peters [:infofrommozilla]]

See also:
Bug 1143744 Table properties don't open by double-click of table cell. Image properties don't open by double-click on right part of image.
Bug 1288929 Double-click in div (cite prefix) or blockquote still opens the Advanced property editor, follow-up from bug 1143744
Bug 1557996 When composing an email, double-clicking on fixed width text <tt> brings up the Advanced Property Editor

Comment 4

[Steph]

I am having trouble to reproduce this bug as well. But I get the Advanced Property Editor. I am replying to an E-Mail with this Text:

E-Mail Text / Content. Lorem Ipsum ...

Mit herzlichen Grüßen
Name Surname

Note, the italic text is the words that will open up the Advanced Property Editor. The other words are behave like they should. (easy highlight to copy / paste)

When looking at the original source of the message, I see this:

  <span style="font-family:&quot;Arial&quot;,sans-serif;mso-fareast-language:DE">
    it herzlichen Grüßen
    <o:p></o:p>
  </span>
</p>
<p class="MsoNormal">
  <span style="font-family:&quot;Arial&quot;,sans-serif;mso-fareast-language:DE">
    Name Surname
    <o:p></o:p>
  </span>
</p>

I hope, this helps.

Comment 5

[Thomas D. (:thomas8)]

(In reply to Steph from comment #4)

I hope, this helps.

Thanks much Steph, this helps a lot! Now easy to reproduce. I have tested this some more and created a reduced testcase.eml (attachment 9294414 [details]).

Comment 6

[Thomas D. (:thomas8)]

Attachment: Testcase 1: 1790388_testcase1_doubleClickWordTriggersAdvancedProperties.eml

Here's a simple testcase reduced from reporter's comment 4.

So Microsoft Office generates an unfortunate set of nested inline/block-level elements.
Double-click on the last word before <o:p></o:p> triggers the unwanted dialog.

<span>double-click HERE<o:p></o:p></span>

Magnus, can you have a look?

Comment 7

[Thomas D. (:thomas8)]

Comment on attachment 9294414 [details]
Testcase 1: 1790388_testcase1_doubleClickWordTriggersAdvancedProperties.eml

.

Comment 8

[Thomas D. (:thomas8)]

Comment on attachment 9294414 [details]
Testcase 1: 1790388_testcase1_doubleClickWordTriggersAdvancedProperties.eml

oops, sorry!

Comment 9

[Magnus Melin [:mkmelin]]

So we have HERE<o:p></o:p> and when HERE is double clicked, the word is selected.
GetObjectForProperties() at https://searchfox.org/comm-central/rev/32ed60f32e390e49cd7763e629eff44a79d857cc/mail/components/compose/content/editor.js#1321 gives us the <o:p> as selected element so we open the advanced editor - https://searchfox.org/comm-central/rev/32ed60f32e390e49cd7763e629eff44a79d857cc/mail/components/compose/content/ComposerCommands.js#1661

So maybe still a bug in the underlying editor. It's both text and <o:p> that are selected so either the <o:p> shouldn't be included in the selection or editor.getSelectedElement() should return null - I would assume.

Comment 10

[wst]

I also have this problem.

Thunderbird: 102.3.2 (32-bit), OS: Win 10.0.19042.2006

Double clicking to select whole word „ABC“ or „XYZ“ (e.g. for copying to clipboard) in compose area in the part of the original message from MS Outlook that is being replied triggers Advanced Property Editor.

Source example:

<ul style=3D"margin-top:0in" type=3D"disc">
<li class=3D"MsoListParagraph" style=3D"margin-left:17.25pt;mso-list:l0 lev=
el1 lfo1">
ABC<o:p></o:p></li><li class=3D"MsoListParagraph" style=3D"margin-l=
eft:17.25pt;mso-list:l0 level1 lfo1">
XYZ<o:p></o:p></li></ul>

Comment 11

[Jay]

Is there hope for a fix for this issue?

It has been driving me crazy for a very long time now. I'm glad to see some background for the reasons this is happening, but is there really no general solution? Can we agree that there should be an option to disable that "Advanced Property Editor on double-click" behavior completely and not just for specific tags?

It's a normal and common workflow to double-click or tripple-click on text so select a word or a paragraph. Thunderbird should not break this by opening a dialog. This behavior should either be removed by default or at least as an option. The context menu is meant to provide access to dialogs like this for selected text.

Comment 12

[Wayne Mery (:wsmwk)]

Certainly a bug, but given the priorities set for the next version I doubt this will be touched by the development team for at least 6 months. But a patch would surely be accepted.

Comment 13

[Steph]

... interesting. I consider this as a very important fix as well. Such a small thing, that could improve working with Thunderbird so much. I wonder, why Apps like postbox-inc.com get it right so easily. This is not meant as a rant, I am just wondering. Unfortunately, I am not able to fix these things myself.

Comment 14

[Jay]

The fix depends on whether that behavior is considered a bug or a feature by the dev team. If they consider this double-click functionality useful for specific cases (although I cannot imagine this to be the case), then they'd need to make this optional.
If they agree that this is simply a bug, because nobody would expect a dialog to be opened by double-clicking on text (more precisely on undefined tags), then all they need to do is to remove the line already pointed out by @mkmelin above):

doAdvancedProperties(element);

https://searchfox.org/comm-central/rev/32ed60f32e390e49cd7763e629eff44a79d857cc/mail/components/compose/content/ComposerCommands.js#1661

Comment 15

[Magnus Melin [:mkmelin]]

See comment 9, would you consider that a bug in nsIHTMLEditor.getSelectedElement, or something we should work around?

Comment 16

[Jay]

While I do indeed think that getSelectedElement returns an incorrect result for cases like HERE<o:p></o:p>, it doesn't really matter for the described problem. Even if the selection would be fixed, the disruptive "Advanced Property Editor" would still come up if the source is <o:p>HERE</o:p>, because it would still fall in the default: doAdvancedProperties(element); break; section. So only fixing getSelectedElement bug won't fix the bug described in this topic.

I would opt for removing the line doAdvancedProperties(element); completely for double-clicked text. If anyone sees any use for this dialog, then it should be moved to the context menu.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

First, HTMLEditor::GetSelectedElement is used only by comm-central. Therefore, it's fine to change the behavior from Firefox point of view.
https://searchfox.org/mozilla-central/search?q=HTMLEditor%3A%3AGetSelectedElement&path=&case=true&regexp=false

And then, it means that comm-central code can implement this by itself (including written in JS). Therefore, I recommend Thunderbird developers port it to JS to make its maintenance cost more reasonable.

Anyway, I checked the behavior. I assume that lastElementInRange is not nullptr at second run of this loop. Then, the following if block should have else block because I think that this block does not assume that there is an empty element except <br> element (HTMLEditor never leaves empty inline elements, it must be the reason).

I guess that it's the best approach here is, clear lastElementInRange if it's not a block, not a replaced element and empty. If we made it return nullptr in this case instead, I think that we'd get opposite bug reports that the advanced dialog never appears for the parent element due to the invisible empty inline element.

Unfortunately, I'm still sick, therefore, I work shorter for a while. Therefore, I don't have much time to work on this now.