Clicking on inserted image in compose window doubles it
Categories
(Thunderbird :: Message Compose Window, defect, P1)
Tracking
(thunderbird78+ fixed)
People
(Reporter: jorgk-bmo, Assigned: rnons)
References
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
Today's Daily TB 78 on Windows.
Start new composition, type some text, capture a bit of your desktop onto the clipboard, insert image into compose window. So far, so good.
Click onto the image. You will see two images. Click anywhere outside the first or second image, the second image disappears.
Looking with ThunderHTMLedit, the second image isn't really inserted, is seems like some display artefact.
Comment 1•5 years ago
|
||
I can see it. I wonder if it's not the zoom-to-full-size/shrink to fit feature somehow behaving badly. Clicking once more it will shrink back to original.
Reporter | ||
Comment 2•5 years ago
|
||
Time to get a regression range, no? Alice, could you please do us the favour?
Comment 3•5 years ago
•
|
||
And no resizer appears.
Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=756d8f7c911959eedf4ecbee51b7e3b3c31f0f99&tochange=ba6ccb3d8aec28505ace35a9e59c86089f38859b
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=853b0e791775ce726149209092a003ed5f001b0c&tochange=a6a5a4f31ea26906e584e162b8d9485e6cacca32
Suspect: bug 1449522
Reporter | ||
Comment 4•5 years ago
|
||
Thank you, Alice, much appreciated. Looks a bit like bug 1449522 or bug 1449522 and friends. Emilio, do you see this in Daily? Any idea?
Reporter | ||
Comment 5•5 years ago
•
|
||
One more thing. What happens at https://www-archive.mozilla.org/editor/midasdemo/ in a FF Daily when you paste a picture there. Same effect?
EDIT: Tried FF Daily of 11 May 2020. No such effect visible there.
Comment 6•5 years ago
|
||
This rule should be hiding that image: https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/editor/composer/res/EditorOverride.css#108
This should insert that sheet: https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/toolkit/content/widgets/editor.js#188-191
Maybe that's running too soon and the about:blank document gets replaced?
Comment 8•5 years ago
|
||
See bug 1636971. Cursor should turn to an arrow when moving over image in composition window, and clicking on image should Iproduce a bounding box with resizing handles, so that image can be visually (manually) resized. As of v 77.0b2, cursor no longer becomes an arrow; it remains as text-editing tool. Clicking on, or dragging cursor across image SOMETIMES selects the image -- and when it does, the duplicate image appears.
It's no longer possible to select the image by clicking on it, or to generate a bounding box.
The problem also occurs whether the image has been pasted into composition window OR using "insert image" menu command or new "insert image" button on toolbar.
Comment 9•5 years ago
|
||
Above comment should say: "See bug 1637836."
Comment 10•5 years ago
|
||
It's also now impossible to select table cells in the compose window -- e.g., when there's a table in a forwarded message (even in the copied header). Is this related to the selection problem in bug 1637836 -- and if so, are these selection problems related to the display-and-selection problems initially noted in this bug?
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
I agree with comment 6, the EditorOverride.css is reverted when loading about:blank. I didn't find how to fix it, this patch just loads EditorOverride.css again in InitEditor
.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 12•4 years ago
|
||
Before bug 1449522, EditorOverride.css
was loaded in HTMLEditor->Init
. And in https://searchfox.org/mozilla-central/source/editor/composer/nsEditingSession.cpp, there is a function call path like this
OnStateChange ---> EndDocumentLoad ---> SetupEditorOnWindow ---> HTMLEditor->Init
I guess OnStateChange
will be called when new URI is loaded, so after loading about:blank, HTMLEditor->Init
will run again to load EditorOverride.css
, that's how it worked before.
Seems to me if this can be fixed from m-c, c-c doesn't need any change. Emilio, do I understand correctly?
Comment 13•4 years ago
|
||
Yeah, that sounds right. Probably this chunk of code needs to be moved to a point where the page has loaded or such.
Comment 14•4 years ago
|
||
Assignee | ||
Comment 15•4 years ago
|
||
I tried
// same error for
// window.addEventListener("load", () => {
document.addEventListener("DOMContentLoaded", () => {
let winUtils = this.contentWindow.windowUtils;
winUtils.loadSheetUsingURIString(
"resource://gre/res/EditorOverride.css",
winUtils.AGENT_SHEET
);
});
but get
JavaScript error: chrome://global/content/elements/editor.js, line 66: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDOMWindowUtils.loadSheetUsingURIString]
I also tried
this.contentWindow.addEventListener("load", () => {
this.contentDocument.addEventListener("DOMContentLoaded", () => {
But they are not triggered at all.
Should I move it back to HtmlEditor.cpp?
Assignee | ||
Comment 16•4 years ago
|
||
Changed to load EditorOverride.css in HTMLEditor->Init
Comment 17•4 years ago
|
||
Assignee | ||
Comment 18•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 19•4 years ago
|
||
Updated•4 years ago
|
Comment 20•4 years ago
|
||
Comment 21•4 years ago
|
||
bugherder |
Assignee | ||
Comment 22•4 years ago
|
||
Comment on attachment 9155924 [details]
Bug 1636971 - Load EditorOverride.css in HTMLEditor->Init to fix image styles in editor. r=emilio
Beta/Release Uplift Approval Request
- User impact if declined: Magnus told me TB78 needs this fix to be uplifted to mozilla-beta.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): No new code added, basically reverted some changes.
- String changes made/needed:
Comment 23•4 years ago
|
||
The user impact is strange doubling of images (etc.) in the Thunderbird compose window. As far as I know there is no impact on Firefox.
Updated•4 years ago
|
Assignee | ||
Comment 25•4 years ago
|
||
I think no impact/risk for Firefox.
Comment 27•4 years ago
|
||
Comment on attachment 9155924 [details]
Bug 1636971 - Load EditorOverride.css in HTMLEditor->Init to fix image styles in editor. r=emilio
I'm afraid that's not a particularly satisfying answer for a late beta uplift.
Comment 28•4 years ago
|
||
Reporter | ||
Comment 29•4 years ago
•
|
||
If you do that, you need to need to back it out from comm-esr78 and land the M-C fix on our branch on mozilla-esr78 instead which is the messier way to do it.
Emilio, can you provide an uplift request that will satisfy Julien, see comment #27.
Comment 30•4 years ago
|
||
(In reply to Jorg K (CEST = GMT+2) from comment #29)
If you do that, you need to need to back it out from comm-esr78 and land the M-C fix on our branch on mozilla-esr78 instead which is the messier way to do it.
Not sure what you mean. Landing attachment 9155187 [details] [diff] [review] on comm-central-beta (only) is all that is needed AFAIK.
Reporter | ||
Comment 31•4 years ago
|
||
So you don't want the "right" solution to go to ESR?
Comment 32•4 years ago
|
||
In one way it doesn't really matter, IMHO.
Comment 33•4 years ago
|
||
Julien, this code is conditional on a flag that is not used by Firefox, is that a more satisfying answer?
Comment 34•4 years ago
|
||
It gets closer, certainly. "I think" and "As far as I know" with no details aren't particularly reassuring, so any extra explanation helps.
Updated•4 years ago
|
Reporter | ||
Comment 35•4 years ago
|
||
Comment 36•4 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Comment 37•4 years ago
|
||
Comment on attachment 9155924 [details]
Bug 1636971 - Load EditorOverride.css in HTMLEditor->Init to fix image styles in editor. r=emilio
Clearing the uplift flag to take this off pending-uplift queries.
Description
•