Reduce the dependency for layout information of table editor #1
Categories
(Core :: DOM: Editor, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox102 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
(Blocks 2 open bugs)
Details
Attachments
(7 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
The table editor (in HTMLTableEditor.cpp
) is used by XPCOM API in comm-central or built-in table editor in Firefox which is disabled by default (web apps can enable it though). So it's a minor feature and perhaps we don't need to take care about backward compatibility so much at least for DOM mutation events. Once we fix bug 1710784, the table editor cannot access the latest layout information after changing the DOM. So, we should make it layout information free as far as possible with flushing pending notifications first.
Assignee | ||
Comment 1•3 years ago
|
||
I don't work on this bug actively in these 7 months, but the patches in my local repository required rebasing a lot. Therefore, I'd like to land the finished patches now.
Assignee | ||
Comment 2•3 years ago
|
||
Our table editor depends on layout information for getting raw/column position
of a cell, checking rawspan/colspan information and table size. Therefore,
they require the latest information, but they don't flush pending layout by
themselves. Therefore, this patch makes them do it by themselves and deleting
unnecessary hack from their tests.
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D146359
Assignee | ||
Comment 4•3 years ago
|
||
It refers the layout information for getting row/column index of selected cell
element or a cell element which contains selection. However, it can stop
referring it if all callers can specify insertion point of new cells, and it's
possible. Therefore, we can make it free from layout information.
Note that this blocks legacy mutation events during inserting table cell
elements, but perhaps this does not cause problems in the wild.
DOMNodeInserted
will be fired after all cell elements are inserted, so
from point of view of the event listeners, editor content is different from
traditional behavior, but this works only when user uses inline table editor
which is disabled by default. I.e., in the wild, this path is rarely run.
Note that the changes are tested by test_nsITableEditor_insertTableColumn.html
and test_nsITableEditor_insertTableCell.html
.
Depends on D146360
Assignee | ||
Comment 5•3 years ago
|
||
This change is tested by test_nsITableEditor_getFirstRow.html
and
test_nsITableEditor_insertTableColumn.html
.
Depends on D146361
Assignee | ||
Comment 6•3 years ago
|
||
There is no direct test because of no corresponding XPCOM method, but this is
called only by HTMLEditor::InsertTableColumnsWithTransaction()
which is
tested by test_nsITableEditor_insertTableColumn.html
. Anyway, the chagne is
really simple.
Depends on D146362
Assignee | ||
Comment 7•3 years ago
|
||
It needs to work with the latest layout information to consider which cell
element is the insertion point due to rowspan and colspan. Therefore,
this patch makes it collects all cell data before touching the DOM except
the case that it needs to normalize the table to make it rectanble.
Note that the case requring the normalizer should be fixed in a later patch.
This method is corresponding to an XPCOM method. Therefore, this is tested
by test_nsITableEditor_insertTableColumn.html
.
And also it's used by the inline table editor, but we don't have automated tests
for this because of no API to get the buttons. Therefore, I tested it by my
hand.
Depends on D146363
Assignee | ||
Comment 8•3 years ago
|
||
Depends on D146364
Comment 10•3 years ago
|
||
Backed out 7 changesets (bug 1730442) for causing reftest failures in editor/reftests/inline-table-editor-position-after-updating-table-size-from-input-event-listener.html
Backout link: https://hg.mozilla.org/integration/autoland/rev/9c5d06519b170fd95fd257aecfb5f3c824aa23fd
Assignee | ||
Comment 11•3 years ago
|
||
Oh, sorry. I've not checked the reftest result in tryserver.
Updated•3 years ago
|
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b4dcdb40970d
https://hg.mozilla.org/mozilla-central/rev/76fac2be98e7
https://hg.mozilla.org/mozilla-central/rev/d51b14fb40ed
https://hg.mozilla.org/mozilla-central/rev/0d370e2abf6d
https://hg.mozilla.org/mozilla-central/rev/e44e3784a8f2
https://hg.mozilla.org/mozilla-central/rev/9282a4bfd86f
https://hg.mozilla.org/mozilla-central/rev/dafaccab50af
Description
•