1843918 - DOMParser.parseFromString on strings without a doctype produces a never-shown console warning (and wastes time doing so)

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Markus Stange [:mstange]]

We can improve the speedometer3 subtest "TodoMVC-JavaScript-ES5" by 6.7% if we don't report a warning for a missing doctype during DOMParser parsing.

This subtest has recently switched to using DOMParser.parseFromString, and the string we feed into it is not a full document, it's just a fragment that was formerly passed to the innerHTML setter. Thus it does not contain a doctype.

You can run the test here: https://shell--speedometer-preview.netlify.app/?suite=TodoMVC-JavaScript-ES5#home

When running the test, I do not see any warnings in the web console. However, the profiler shows time spent reporting a console warning:

https://share.firefox.dev/3DevefZ 1893 samples in nsHtml5DocumentBuilder::SetDocumentMode
https://share.firefox.dev/3XU7NSA 2024 samples in LogMessageRunnable::Run

Together this makes up 6.7% of the sample count on this subtest.

Can we skip the warning during DOMParser.parseFromString parsing?

Comment 1

[Markus Stange [:mstange]]

https://searchfox.org/mozilla-central/rev/7a4c08f2c3a895c9dc064734ada320f920250c1f/parser/html/nsHtml5DocumentBuilder.cpp#74-113

void nsHtml5DocumentBuilder::SetDocumentMode(nsHtml5DocumentMode m) {
  nsCompatibility mode = eCompatibility_NavQuirks;
  const char* errMsgId = nullptr;

  switch (m) {
    case STANDARDS_MODE:
      mode = eCompatibility_FullStandards;
      break;
    case ALMOST_STANDARDS_MODE:
      mode = eCompatibility_AlmostStandards;
      errMsgId = "errAlmostStandardsDoctypeVerbose";
      break;
    case QUIRKS_MODE:
      mode = eCompatibility_NavQuirks;
      errMsgId = "errQuirkyDoctypeVerbose";
      break;
  }
  mDocument->SetCompatibilityMode(mode);

  if (errMsgId) {
    nsCOMPtr<nsIURI> docURI = mDocument->GetDocumentURI();
    bool isData = false;
    docURI->SchemeIs("data", &isData);
    bool isHttp = false;
    docURI->SchemeIs("http", &isHttp);
    bool isHttps = false;
    docURI->SchemeIs("https", &isHttps);

    nsCOMPtr<nsIPrincipal> principal = mDocument->GetPrincipal();
    if (principal->GetIsNullPrincipal() && !isData && !isHttp && !isHttps) {
      // Don't normally warn for null principals. It may well be internal
      // documents for which the warning is not applicable.
      return;
    }

    nsContentUtils::ReportToConsole(
        nsIScriptError::warningFlag, "HTML_PARSER__DOCTYPE"_ns, mDocument,
        nsContentUtils::eHTMLPARSER_PROPERTIES, errMsgId);
  }
}

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I think we should just change https://searchfox.org/mozilla-central/rev/7a4c08f2c3a895c9dc064734ada320f920250c1f/parser/html/nsHtml5DocumentBuilder.cpp#93 to
if (errMsgId && !mDocument->IsLoadedAsData())

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1843918, don't even try to warn about compatibility mode when parsing a data document, r=#dom-core

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca34c6c945f9
don't even try to warn about compatibility mode when parsing a data document, r=dom-core,edgar

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ca34c6c945f9