739805 - Calling getTabState() on a not-yet-restored tab wipes out text and scroll data

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Calling getTabState() on a tab that is restoring or waits to be restored wipes out text and scroll data contained in the tab state. _updateTextAndScrollDataForTab() shouldn't really do anything when the tab isn't restored, yet.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 609914 [details] [diff] [review]
patch v1

Review of attachment 609914 [details] [diff] [review]:
-----------------------------------------------------------------

I may have gotten a bit picky about the test, but I want to make sure it tests the right thing! Fix or let me know it's fine as is.

::: browser/components/sessionstore/test/browser_739805.js
@@ +15,5 @@
> +  whenBrowserLoaded(browser, function () {
> +    whenTabRestored(tab, function () {
> +      let input = browser.contentDocument.getElementById("foo");
> +      is(input.value, "bar", "formdata has been restored correctly");
> +      finish();

Will we ever get to here before the checks below are run? setTabState goes pretty directly to loading a tab (unless restore on demand).

Perhaps it would be more explicit if we ensure tab isn't selected, restore_on_demand is on, doing the checks, then selecting & checking the form value.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Paul O'Shannessy [:zpao] from comment #2)
> Perhaps it would be more explicit if we ensure tab isn't selected,
> restore_on_demand is on, doing the checks, then selecting & checking the
> form value.

Yeah, that seems better to me as well. Fixed the test to match your suggestion.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/3c8f6fe5ce4e

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/3c8f6fe5ce4e