Closed Bug 1268736 Opened 4 years ago Closed 3 years ago

Too hard to select text inside table cell (Contenteditable on page messes up with table - round 2, fight)

Categories

(Core :: Editor, defect)

47 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- verified

People

(Reporter: arni2033, Assigned: bugs)

References

Details

(Keywords: regression, ux-consistency)

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #974309 +++

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160426044609
STR:
1. Copy the following "data:" url, then open new tab, paste it in urlbar, press Enter
>   data:text/html,<body onload="F.focus()"><table><tr><td>AB1CD<tr><td>AB2CD</table><table><td><div contenteditable id="F">F</div></table><div></div>
2. Try to select "B1C" with mouse once
3. Click on "1" on the page
4. Try to select "B2C" with mouse once
5. Click on "2" on the page
6.(unnecessary) Repeat Steps 2-5

AR:  Can't select text
ER:  Text should be easily selected in Step 2

Note:
 I encounter it on https://vk.com/ , in "personal messages". They have table design and 
 automatically focus entry field, just like my reduced testcase.
 
This is regression from bug 974309. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ee4201100b15c95ca409faee977f45d39f7e52a1&tochange=71b0b49c6d47bdb2fec86bb750fad2e855fceb9a
It became worse than before bug 974309: now it's too hard to even select text in _one_ table cell.
Please, back out patch in bug 974309 and develop a better solution.
Flags: needinfo?(bugs)
(In reply to arni2033 from comment #1)
> It became worse than before bug 974309: now it's too hard to even select
> text in _one_ table cell.
> Please, back out patch in bug 974309 and develop a better solution.

I was able to reproduce this issue. I'll post an updated fix in bug 974309.
Flags: needinfo?(bugs)
Assignee: nobody → bugs
Status: NEW → ASSIGNED
Attachment #8755657 - Flags: review?(mats)
Comment on attachment 8755657 [details] [diff] [review]
An editable table cell means that both the cell and its contents are children of an editable node.

Looks reasonable to me.  r=mats

Nits:
Please move the nsContentUtils::ContentIsDescendantOf call to the next
line to avoid making the line so long.

The #ifdef/#endif should not be indented.

The "if (mCellParent)" check is now redundant; please remove it
to avoid any confusion.

It might also make the intent of the code clearer if we rename
"editable" to "editableCell".

Also, we really should write a regression test for this.
Attachment #8755657 - Flags: review?(mats) → review+
Pushed by jvillegas@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5a74fc98333
An editable table cell means that both the cell and its contents are children of an editable node. r=mats
https://hg.mozilla.org/mozilla-central/rev/d5a74fc98333
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Should we consider this for a Beta uplift?
Flags: needinfo?(bugs)
Version: unspecified → 47 Branch
Yes, thanks Ryan.
Flags: needinfo?(bugs)
Too late for 48. Jet, next time, please fill the uplift request directly...
Verified fixed FX 49b8 on Win 7, Ubuntu 14.04, OS X 10.11.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.