1753588 - Slow Loading of large HTML Tables since Version 94

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[craig]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:93.0) Gecko/20100101 Firefox/93.0

Steps to reproduce:

We have a large HTML table with about 200 rows, and 10 columns. The script is pulling a lot of data but generally takes about 3-5 seconds to load. This is on v93. Since v94 this has changed to over 1 minute. Thought it was our site until I went back to v93 on which it works.

Actual results:

I opened a thread on FireFox Support https://support.mozilla.org/en-US/questions/1364203. Was advised to use mozregrssion which I did. The results are on the thread. The help suggestion (Until it's fixed you can enter about:config in the URL bar and set "layout.css.cascade-layers.enabled" = false.) did not work although it did improve it.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

That regression range makes no sense, cascade layers are shipping in Firefox 97, and weren't implemented in Firefox 94.

Can you attach about:support? Do you have a URL that reproduces the issue? The 3s vs. minutes difference is massive.

Comment 3

[craig]

Bear in mind I am only a user.

What is about:support. Which version(s) do you want it from.

It is happening inside of our CRM. It would be difficult to provide you with access. I am happy to film it or provide you with HTML?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

On a slow version, can you navigate to about:support (typing it in the url bar), click "copy text to clipboard", and paste it here? That'll attach it to this bug. If you also do it for a fast version it'd be great.

If you have some HTML page that reproduces the issue, yeah, by all means, that'd be super helpful, thanks! :)

Comment 5

[craig]

Attachment: Version 93: Fast Version

Comment 6

[craig]

Attachment: Version 96: Slow

Comment 7

[craig]

Attachment: HTML Source of the Page

Comment 8

[craig]

Attachments created.

Comment 9

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

https://share.firefox.dev/3J31ovD

Something to do with LoginManagerHelper.jsm

Comment 10

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

Or has something made .labels slower.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Can you confirm going to about:config and setting signon.usernameOnlyForm.enabled=false makes it fast again on the original page?

Comment 12

[craig]

Excellent. I can confirm that when I changed signon.usernameOnlyForm.enabled from FALSE to TRUE the table rendered within 3 seconds on the v96 machine.

Comment 13

[craig]

Sorry last post is incorrect. I will correct.

Comment 14

[craig]

When signon.usernameOnlyForm.enabled=false then loads very fast. About 3 seconds.
When signon.usernameOnlyForm.enabled=true then loads very slowly. Over 60 seconds.

This is on the v96 machine. I have tested both settings twice to confirm.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Thanks for confirming!

Comment 16

[craig]

My pleasure. Glad I can help.

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1732901

Comment 18

[Dimi Lee [ooo 16.04 - 21.04］]

We should have a limit regarding the maximum number of form we should process (infer whether this is a username-only form) in the password manager.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

I don't think this is necessarily about the number of forms... Checking the labels is pretty expensive. Maybe we should make .labels faster at least for connected docs by keeping a list of HTMLLabelElements in DocumentOrShadowRoot, wdyt Olli?

Comment 20

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

Hmm, tracking all the changes might get a bit hairy, no?
Basically implement nsContentList in a new way, or did you have something else in mind?

Dimi, does the max number for forms work in this case?
The example page has 14498 form elements and 14501 input elements.
Even if we optimized .labels a lot, going through all those elements would take lots of time.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

(In reply to Olli Pettay [:smaug] from comment #20)

Hmm, tracking all the changes might get a bit hairy, no?
Basically implement nsContentList in a new way, or did you have something else in mind?

I was thinking of something like what we do for <meta name=color-scheme> here, and yeah, tweaking nsLabelsNodeList to look at that when our owner element is connected so we don't have to iterate over it all. But yeah I hadn't realized that there are 14k forms.

Maybe just not going through them all makes it fast enough. But it's sad that .labels needs to do a full-document traversal :)

Comment 22

[Dimi Lee [ooo 16.04 - 21.04］]

(In reply to Olli Pettay [:smaug] from comment #20)

Dimi, does the max number for forms work in this case?
The example page has 14498 form elements and 14501 input elements.
Even if we optimized .labels a lot, going through all those elements would take lots of time.

Yes, reducing form processed while detecting multi-page login form is something I want to do. From telemetry, roughly 90% cases are under 5 forms (here form refers to forms that are possible a login form) per document. Since it's not likely a "login page" contains a lot of forms. I would like to start with setting the threshold to 5 so our heuristic can still cover most of the cases. I guess this optimization and .labels optimization can be done in parallel if we both think they are the right thing to do?

Comment 23

[Dimi Lee [ooo 16.04 - 21.04］]

Attachment: Bug 1753588 - Restrict the number of form looked up while checking whether autofilling a username-only form r=tgiles

This patch adds a pref "signon.usernameOnlyForm.formThreshold" to limit
the number of form we processed while receiving "PWMGR_NUM_FORM_HAS_POSSIBLE_USERNAME_EVENT_PER_DOC"
event. This improves the performance while loading a page with multiple username-only likely form.

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1732901

Comment 25

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

(In reply to Dimi Lee [ooo Jan.29 - Feb.5] from comment #22)

and .labels optimization can be done in parallel if we both think they are the right thing to do?

(It is not clear to me what ".labels optimization" actually would do)

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5936869f553d
Restrict the number of form looked up while checking whether autofilling a username-only form r=tgiles,sgalich

Comment 27

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing failures on test_basic_form_honor_autocomplete_off.html

Backout link : https://hg.mozilla.org/integration/autoland/rev/df3ebc33eca8d286cf5eef243463aa2aa2078319

Push with failures : https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=5936869f553ddf0b9d1d12e7f0914997c989e01f&selectedTaskRun=cxI64Q4JQv2oEG9fvrwKUg.0

Link to failure log : https://treeherder.mozilla.org/logviewer?job_id=367524790&repo=autoland&lineNumber=11509

Failure message: TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/mochitest/test_basic_form_honor_autocomplete_off.html | Test timed out.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e86754f4d3fe
Restrict the number of form looked up while checking whether autofilling a username-only form r=tgiles,sgalich

Comment 29

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 30

[Monica Chiorean]

I tried to reproduce the issue on Win10x64 before the fix using build 94.0a1(20210920212918) and after the fix 99.0b8 (20220324185704) but loading of "https://bugzilla.mozilla.org/attachment.cgi?id=9262305" is around 40sec - 1minute, so it is not loading in 3-4sec like you mentioned.

Can you please confirm if the issue is still reproducing on your sided using latest Beta (https://archive.mozilla.org/pub/firefox/candidates/99.0b8-candidates/)? Thank you.

Comment 31

[craig]

I have just installed 99.0b8 and tested it on the table that I first reported problems with and it is working as expected. Load time for table with 2219 rows about 4 seconds. Seems fine.

Comment 32

[Monica Chiorean]

Mark as verified per above comment.