Closed Bug 1670137 Opened 4 years ago Closed 4 years ago

Re-enable test_formless_submit_navigation_negative.html in cross-origin mode

Categories

(Toolkit :: Password Manager, defect, P2)

defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: enndeakin, Assigned: annyG)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

From bug 1661266, comment 9:

This test test_formless_submit_navigation_negative.html loads a form and then navigates through history using history.back() and history.go(-1) and expects to not save filled in passwords in these cases. Normally, at

https://searchfox.org/mozilla-central/rev/1a973762afcbc5066f73f1508b0c846872fe3952/toolkit/components/passwordmgr/LoginManagerChild.jsm#171

the value of webProgress.loadType is checked to see whether the load is due to a history navigation and returns early if so. With cross-origin mode disabled, the value is correctly LOAD_CMD_HISTORY.

With cross-origin mode enabled, the value is instead LOAD_CMD_NORMAL, so the form is saved instead and the popup appears, causing the test to fail.

So the issue here is likely that the web progress loadType is not being set correctly.

Reassigning needinfo, as I don't know who would be best able to fix this.

(Note: the other test test_formless_submit_navigation.html does indeed now work and could be enabled)

No longer depends on: 1661266
Assignee: nobody → agakhokidze
Status: NEW → ASSIGNED

After debugging this for some time, it looks like the problem happens not when we are going back -1 or reloading the page, but when we try to reset the page for the iframe to blank.html here before doing those steps.

Here are some logs from when we have a first failing assertion:

 1:52.53 INFO add_task | Entering test testcase-2-HISTORY_GO_MINUS1
 1:52.53 INFO Starting testcase with script HISTORY_GO_MINUS1: {"document":"<input type=password value=\"\">","selectorValues":{"[type=password]":"pass2"},"wouldCapture":true}
...
 1:52.97 FAIL No formSubmissionProcessed should occur in this test
    SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
    submissionProcessed@toolkit/components/passwordmgr/test/mochitest/test_formless_submit_navigation_negative.html?currentTestURL=toolkit%2Fcomponents%2Fpasswordmgr%2Ftest%2Fmochitest%2Ftest_formless_submit_navigation_negative.html&closeWhenDone=1&showTestReport=true&expected=pass:23:5
 1:52.98 INFO got: [{"origin":"http://test1.mochi.test:8888","data":{"browsingContextId":10737418241,"formActionOrigin":"http://test1.mochi.test:8888","usernameField":null,"newPasswordField":{"name":"","value":"pass2"},"oldPasswordField":null,"dismissedPrompt":false,"possibleValues":{"usernames":{},"passwords":{}},"messageSent":true}}]
...
 1:53.15 GECKO(11074) [Child 11245: Main Thread]: D/nsDocShell nsDocShell[0x7f510ec5f400]::OnNewURI("http://test1.mochi.test:8888/tests/toolkit/components/passwordmgr/test/mochitest/blank.html", [http://test1.mochi.test:8888/tests/toolkit/components/passwordmgr/test/mochitest/blank.html], 0x1)
 1:53.16 GECKO(11074) [Child 11245: Main Thread]: D/nsDocShell   shAvailable=1 updateSHistory=1 updateGHistory=1 equalURI=1
 1:53.28 INFO setUserInputValues, selector: [type=password]
 1:53.31 INFO Running HISTORY_GO_MINUS1 script to check for a submission

As you can see, the assertion fails before line 1:53.28 INFO setUserInputValues, selector: [type=password] where we set the form values for this test case and before line 1:53.31 INFO Running HISTORY_GO_MINUS1 script to check for a submission where we run the script (in this case history.go(-1);).

The previous test case for the same type of form, and the script executed being history.back();, passes. So if we recreate some of the steps that happened before unexpected "formSubmissionProcessed" message was dispatched, here is what happened:

  1. Previous test case starts - testcase-2-HISTORY_BACK - for the same form type {"document":"<input type=password value=\"\">","selectorValues":{"[type=password]":"pass2"},"wouldCapture":true}
  2. iframe's src is set to http://test1.mochi.test:8888/tests/toolkit/components/passwordmgr/test/mochitest/blank.html (as a way of resetting I suppose)
  3. we fill out selector values in the form
  4. we execute HISTORY_BACK script inside of the iframe - history.back();
  5. history.back() does not take place because we find no differences between entries (LoadDifferingEntries keeps returning false)
  6. no "formSubmissionProcessed" messages have been received during any of the steps
  7. testcase-2-HISTORY_GO_MINUS1 starts, note there is still form data filled in from the previous test case
  8. iframe is navigated to http://test1.mochi.test:8888/tests/toolkit/components/passwordmgr/test/mochitest/blank.html.
  9. we get "formSubmissionProcessed"

Some things to explore:

  • Why does history.back() fail, and is that expected? When debugging I see that none of the entries (total of 8) have any children entries, even though all correspond to frame navigations.
    • Set a breakpoint in parent process at the point when we receive a command for navigating back in history, and then from there set a breakpoint in nsSHistory::AddEntry and reverse-continue until I get to the last place when we added an entry to mEntries array (8th entry)
    • When we are about to add an entry (before we navigate back), in nsSHistory::CloneAndReplaceChild we don't end up walking history entries because current entry (7th entry) has no children and we don't add the entry for the iframe because data->destTreeParent has no children.
    • To fix this, we should go back to the very first time when we try to add an sh entry corresponding to an iframe and see if we add children to a root sh entry
  • TODO do we ever add an sh entry for the iframe we loaded initially?
  • Skip back to when we are doing pushState and end up in parent process - CanonicalBrowsingContext::SetActiveSessionHistoryEntry. We end up calling this

We have the following setup: top level window which contains a test harness subframe (subframe 1) which contains an iframe (subframe 2) which innerHTML we modify with various form types.
After putting some extra logs and prints and debugging, this is what I observed:

  • Before we start the test setup, SH entry for top level window adds a child SH entry corresponding to subframe 1 with uri about:blank
  • That subframe is later removed, because of this call https://searchfox.org/mozilla-central/rev/e0eb861a187f0bb6d994228f2e0e49b2c9ee455e/dom/base/nsFrameLoader.cpp#1880 and I'm unsure why it says it is most likely a dynamically created iframe in the comments, because subframe 1 is not a dynamically created iframe
  • Yet the removed subframe remains to be the mActiveEntry in the canonical browsing context, despite being removed
  • Then later when we are loading a mochitest url into subframe 1, we use id of the removed SH entry as a guide for which entry we need to replace when we clone the current SH entry, but we don't end up finding it because that entry has been removed, and so we don't add SH entry for subframe 1 as a child entry for top level SH entry
Pushed by agakhokidze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f65ad9a67458 Don't remove sh entries for subframes that are switching processes, r=peterv
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: