1411396 - [e10s] Form control required popup doesn't follow input when scrolling

Status: RESOLVED DUPLICATE of bug 643758

(Firefox :: General, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

Not sure if this is the right component...

A friend of mine reported this to me today.

It seems that in other browsers, when you try to submit the form pressing enter, and then scroll, the "Please fill out this field" popup follows the input.

So, STR:

 * Open attached test-case.
 * Focus the text input, then press enter. A "Please fill out this field" popup appears.
 * Scroll down the page, without unfocusing the field.

Expected:

 * The popup remains under the form input.

Actual:

 * The popup stays in the same position, which hides other elements of the page, and is ugly.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: testcase.html

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: screenshot.png

Comment 3

[Emilio Cobos Álvarez (:emilio)]

So when looking at it, this is all handled by FormSubmitObserver / FormValidationHandler, so this seems to be a Firefox bug, not a Core bug.

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Matt, is this a known bug?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I shall point out that this works fine in Chrome and Edge, fwiw.

Comment 6

[Matthew N. [:MattN]]

I've never worked on this code before (felipe has) but I see that it's an e10s regression. The "scroll" event doesn't fire on a <browser> with e10s on: https://dxr.mozilla.org/mozilla-central/rev/1c618b1a13662de7cec429f606367db3827b6dc7/browser/modules/FormValidationHandler.jsm#70-71

Comment 7

[Matthew N. [:MattN]]

Attachment: Bug 1411396 - WIP: Close form validation popups upon scrolling with e10s

Review commit: https://reviewboard.mozilla.org/r/194768/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194768/

Comment 8

[Matthew N. [:MattN]]

Great… in case it wasn't obvious that this feature isn't really maintained… I found out that two of three tests which references this popup were disabled in bug 1057615.

Comment 9

[Matthew N. [:MattN]]

Attachment: Bug 1411396 - Cleanup and move formValidation b-c tests.

Review commit: https://reviewboard.mozilla.org/r/194776/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194776/

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8923654 [details]
Bug 1411396 - Cleanup and move formValidation b-c tests.

https://reviewboard.mozilla.org/r/194776/#review199838

::: browser/modules/test/browser/formValidation/browser.ini:3
(Diff revision 1)
> +[browser_form_validation.js]
> +skip-if = true # bug 1057615
> +[browser_validation_iframe.js]

I enabled this test since I think it should pass with the iframe height change that was caused by scroll in non-e10s closing the popup.

Comment 11

[Matthew N. [:MattN]]

Comment on attachment 8923642 [details]
Bug 1411396 - WIP: Close form validation popups upon scrolling with e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/194768/diff/1-2/

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 8923642 [details]
Bug 1411396 - WIP: Close form validation popups upon scrolling with e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/194768/diff/1-2/

Comment 13

[Matthew N. [:MattN]]

I thought this was going to be trivial but the with the tests being broken and the fact that we need to handle a scroll caused by a validation message, I don't have time to look at this now. I've attached my WIP patch with the TODO of what needs to be fixed. 

We really should have a module to handle laying out UI in the parent process that is supposed to be attached to content elements. We would use that module for autocomplete, <select> popups, and form validation rather than each implementing it differently and with potential security bugs. I'll file a separate bug for this.