Closed Bug 1706266 Opened 9 months ago Closed 3 months ago

Stop supporting Gecko specific commands of `Document.execCommand` if possible

Categories

(Core :: DOM: Editor, task, P3)

task

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

  • increasefontsize
  • decreasefontsize
  • gethtml
  • heading
  • contentReadOnly
  • readonly
  • insertBrOnReturn
  • saveas

are not defined as editor commands by Chromium.

So, once we're sure that they are not used by our users' web apps, we can drop supporting them.

Severity: -- → S3
Priority: -- → P3

In beta 94:

increasefontsize

decreasefontsize

gethtml

  • No using data in beta 94, 93, 92, so that this is never used even as technical examples

heading

contentReadOnly

readonly

  • No using data in beta 94, 93, 92, so that this is never used even as technical examples

insertBrOnReturn

According to these results, we can unship increasefontsize, decreasefontsize, gethtml, heading, readonly safely.

However, I'm not sure about contentReadOnly and insertBrOrReturn. They have been supported only by Gecko. And the impact of using them is not trivial. So that it's hard to use them in web apps which support multiple browsers. All of their usage count is about 10k except querying enabled/supported state. This means that this is not caused by loading the example of MDN because execution count must be bigger than the number of query for disabling the buttons in it. Additionally, I forgot to put a probe to count executing insertBrOnReturn. So perhaps we should keep them for now.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

. Additionally, I forgot to put a probe to count executing insertBrOnReturn.

Ah, I forgot the reason, it's because of used internally.
https://searchfox.org/mozilla-central/rev/4f9bbbe5487da6d1c3680488e016f7bb0cbaa128/dom/base/Document.cpp#6102

It's a DOM API and we are going to change supporting commands for making it
compatible with the other browsers.

Currently, it's used only in Document::EditingStateChanged() to initialize the
default paragraph separator from <br> to <p> or <div>. However, the
command, insertBrOnReturn is Gecko specific one, and I'd like to stop
supporting it with the following patch. Therefore, we need to do this change
first.

For mozregressions, this patch will be landed separately before landing the
following patch.

The telemetry result is written in bug 1706266 comment 1:
https://bugzilla.mozilla.org/show_bug.cgi?id=1706266#c1

increasefontsize, decreasefontsize, gethtml, heading and readonly are
obviously not used by web apps in the wild. Therefore, they can be disabled
in all channels.

contentReadOnly and insertBrOnReturn are odd. The usage is really low (less
than 1% of beta users). However, the number of documents which used the command
is about 1k samples. The result of the commands are not tiny (making the editor
not editable or changing the behavior at typing Enter key in <div>, <p>,
etc). Therefore, it's hard to use them in web apps which supports not only
Gecko. So I guess that they are collected the number of used by automated
tests of somebody because of the constant number in other beta versions.
Perhaps, we should disable it only in Nightly channel for now, and after a
couple of releases, we should try to disable those commands too later.

Depends on D130328

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6f0d89c7a869
part 1: Make nobody use `Document::ExecCommand()` internally r=smaug
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5d6a85b1788e
part 2: Disable some Gecko specific edit commands in release and beta channel, and all of them in nightly channel r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31519 for changes under testing/web-platform/tests
Regressions: 1739663
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.