The default bug view has changed. See this FAQ.

Table properties don't open by double-click of table cell. Image properties don't open by double-click on right part of image.

RESOLVED FIXED in Thunderbird 50.0

Status

MailNews Core
Composition
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: Bernd, Assigned: Jorg K (GMT+1))

Tracking

({regression})

Thunderbird 50.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird47 wontfix, thunderbird48 fixed, thunderbird49 fixed, thunderbird50 fixed, thunderbird_esr45 wontfix)

Details

(Whiteboard: [regression:TB31])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305021524

Steps to reproduce:

Thunderbird v. 31.5.0
Opened a HTML- composition window.
Insert a table (insert > table..) > OK.
Doubleclicked into an empty field of the table.



Actual results:

No reaction.


Expected results:

It should be opened the "table properties". 
This happened still in v 24.1 (on my old portable version).
An other user maintaines that it doesn't work only since v. 31.3.

You can reach the properties menu problem-freeclicking on a table cell via > format > table > table properties.
Jorgk, any idea if this was an intentional change?
Component: Untriaged → Composition
Flags: needinfo?(mozilla)
Product: Thunderbird → MailNews Core
Summary: Table properties don't open by doubleclick → Table properties don't open by doubleclick of table cell
Version: 31 Branch → 31
(Assignee)

Comment 2

10 months ago
Wayne, thanks for alerting us to this bug ;-)

This is a regression from
https://hg.mozilla.org/comm-central/rev/107302e430f1
It says that it's bug 525125, but that's clearly wrong. I found the bug by searching for "107302e430f1" in a comment: Bug 525124 which landed on TB 31.
I backed it out locally and the table properties come up again when double-clicking behind the content of the cell.

Also, the image properties come up reliably when backing this out, see bug 1246094 which has caused us a lot of headache.

I think that change smashed a lot of things :-(
I don't know what is the lesser evil: The problem of bug 525124 that the Advanced Property Editor is coming up or smashing all other special double-click actions. Grrrrr.

What Neil did there is really bad. Aceman, how can we fix this? Two experts reviewed this so three people failed here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mozilla) → needinfo?(acelists)
Keywords: regression
(Assignee)

Comment 3

10 months ago
I don't understand why they didn't land Magnus' patch back in bug 525124: Attachment 8402391 [details] [diff].

That did exactly what was needed.
I'm out of my element here, but I'm wondering whether double click _should_ open properties
Blocks: 525124
Whiteboard: [regression:TB31]
(Assignee)

Comment 5

10 months ago
Created attachment 8762297 [details] [diff] [review]
Backing out Neil's fix from bug 525124 and using Magnus' approach instead.

Magnus, look what Neil & Co. have broken here. Your patch was clear and straight forward and they just ignored it. And now, 19 versions later, we need to go in and fix it again after it caused so many other problems. Sigh.

NI: Just for your information.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(acelists) → needinfo?(mkmelin+mozilla)
Attachment #8762297 - Flags: review?(acelists)
(Assignee)

Updated

10 months ago
Summary: Table properties don't open by doubleclick of table cell → Table properties don't open by double-click of table cell. Image properties don't open by double-click on right part of image.
(Assignee)

Updated

10 months ago
Blocks: 1246094
(Assignee)

Comment 6

10 months ago
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #4)
> I'm out of my element here, but I'm wondering whether double click _should_
> open properties

Yes, it should. And double-clicking an image should also open the image properties. It always did until it got smashed. So let's un-smash it now!

Comment 7

10 months ago
So this is why I couldn't get Table properties to open when debugging the editor dialogs recently? :)

But other dialogs worked, why is only the table element affected? Is it because the table contains the other elements which may also have their own dialogs (that we want access too) ?
(Assignee)

Comment 8

10 months ago
It's because table elements are followed by <br>, use ThunderHTMLedit to see it.

I'm taking the liberty to land this now. I am reviewing Magnus' patch and I'm giving it r+ ;-)
(Assignee)

Comment 9

10 months ago
Comment on attachment 8762297 [details] [diff] [review]
Backing out Neil's fix from bug 525124 and using Magnus' approach instead.

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

This is Magnus' patch from bug 525124, it is clear and simple and I have the right to review it.
r=jorgk.
Attachment #8762297 - Flags: review?(acelists) → review+
(Assignee)

Comment 10

10 months ago
https://hg.mozilla.org/comm-central/rev/862885de2876

As I said: I reviewed the attachment 8402391 [details] [diff] [review] by Magnus from bug 525124 and found it good.
I am also backing out Neil's faulty fix which has caused this regression which initially started with table properties not opening, but in fact also affected image properties.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
Flags: needinfo?(mkmelin+mozilla)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0

Comment 11

10 months ago
Comment on attachment 8762297 [details] [diff] [review]
Backing out Neil's fix from bug 525124 and using Magnus' approach instead.

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

You can get my support :)

This allows me to open table properties again. Also an image inside table works.
Also it seems you can click anywhere in the image to get its properties (not only in the first half of it, as discussed in bug 1246094).

But I'll leave a request to Neil here, maybe he can find out why this solution didn't work for him (or Seamonkey).
Attachment #8762297 - Flags: review+
Attachment #8762297 - Flags: feedback?(neil)

Updated

10 months ago
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Comment 12

10 months ago
Thanks. Now six eyes have looked at it ;-)
(Assignee)

Comment 13

10 months ago
Comment on attachment 8762297 [details] [diff] [review]
Backing out Neil's fix from bug 525124 and using Magnus' approach instead.

[Approval Request Comment]
Regression caused by (bug #): Bug 525124.
User impact if declined: Table and Image properties don't open on double-click.
Testing completed (on c-c, etc.): Manual.
Double-click on image is also tested by test-image-insertion-dialog.js.
Risk to taking this patch (and alternatives if risky):
Low risk. Backed out faulty fix and landed very local fix instead.
Attachment #8762297 - Flags: approval-comm-esr45?
Attachment #8762297 - Flags: approval-comm-beta+
Attachment #8762297 - Flags: approval-comm-aurora+
(Assignee)

Updated

10 months ago
status-thunderbird47: --- → wontfix
status-thunderbird48: --- → affected
status-thunderbird49: --- → affected
status-thunderbird50: --- → fixed
status-thunderbird_esr45: --- → affected

Comment 14

10 months ago
Actually "The problem of bug 525124 that the Advanced Property Editor" was not a problem for me at all. but instead, a convenient way to open the advanced editor. (now it's much more difficult)
Wondering if that behavior is restored (will test when I update trunk.
(Assignee)

Comment 15

10 months ago
No, it's now restored. I couldn't regress the other bug. Now if you double-click in "free space" nothing will happen. It's just implemented some other way.
(Assignee)

Comment 16

10 months ago
Typo: No, it's NOT restored.
(Assignee)

Comment 17

10 months ago
Aurora (TB 49):
https://hg.mozilla.org/releases/comm-aurora/rev/a29b0441a710
status-thunderbird49: affected → fixed
(Assignee)

Comment 18

10 months ago
Beta (TB 48):
https://hg.mozilla.org/releases/comm-beta/rev/aaf03f68fdd2
status-thunderbird48: affected → fixed
(Assignee)

Comment 19

8 months ago
Kent, this wasn't quite right, so if you land this on ESR45, please include a further fix from bug 1288929.
(Assignee)

Updated

8 months ago
Depends on: 1288929

Comment 20

8 months ago
Comment on attachment 8762297 [details] [diff] [review]
Backing out Neil's fix from bug 525124 and using Magnus' approach instead.

I'm not convinced that Magnus's solution is ideal, but then my editor glasses have a different colour to his Thunderbird glasses, and you don't worry about All Tags mode for instance, which you might have broken for us. Also, I'm not sure what behaviour you get if you insert a heading inside a table cell.

Anyway, I agree that rangeParent isn't what we want here, because it isn't what I thought it was at the time. Perhaps what we should be doing is to call GetObjectForProperties()? (That's the function used to populate the Format - Advanced Properties menuitem.)
Attachment #8762297 - Flags: feedback?(neil)

Comment 21

6 months ago
Hmmm, the first patch was "low risk" but somehow risky enough to introduce a regression that had to be fixed in a followup bug. There is also a clear alternative, right? (Format/Table/Table Properties...)

I'm not seeing any compelling reason to take this in 45.4.0
status-thunderbird_esr45: affected → wontfix

Updated

6 months ago
Attachment #8762297 - Flags: approval-comm-esr45? → approval-comm-esr45-
You need to log in before you can comment on or make changes to this bug.