Focused element's state not saved in session history ###!!! ASSERTION: Wrong root: '(nsContentUtils::ContentIsDescendantOf...

RESOLVED FIXED

Status

()

Core
DOM: Core & HTML
--
major
RESOLVED FIXED
12 years ago
5 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: sicking)

Tracking

({dataloss, regression})

Trunk
dataloss, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
This is apparently a regression from bug 358106.

Steps to reproduce problem:
1. Disable bfcache
2. Open a page containing a text control e.g. data:text/html,<input>
3. Modify one of the controls, and leave focus in the control
4. Use back/forward buttons to navigate away from the page and back again

Expected result: control state stored in session history

Actual result: control value lost

Additional information:
###!!! ASSERTION: Wrong root: '!aRoot || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::ContentIsDescendantOf(aEndN, aRoot))', file c:/trunk/mozilla/content/base/src/nsRange.cpp, line 405

Note that although it is not necessary to disable bfcache to trigger the assertion it is required to reliably demonstrate the session history dataloss.
With bfcache enabled all you have to do is navigate some in other tabs/windows to cause dataloss...

We need to fix this, imo.  And we're sure this (not the assert, the dataloss) is a regression from bug 358106?
Flags: blocking1.9?
Uh, I may have filed bug 359919 as a dupe for the assertion... oops :(

Re comment 1: in bug 359919, I clearly showed the new assertion lines (for ContentIsDescendantOf) is what fails.
Summary: Focused element's state not saved in session history → Focused element's state not saved in session history ###!!! ASSERTION: Wrong root: '(nsContentUtils::ContentIsDescendantOf...
*** Bug 359919 has been marked as a duplicate of this bug. ***
Created attachment 245048 [details] [diff] [review]
Patch to fix

This fixes the assertion and seems to fix the dataloss too. I'll dig a bit further to see why serializing the range failed since the assertion in this case was actually bogus.
Assignee: traversal-range → bugmail
Status: NEW → ASSIGNED
Attachment #245048 - Flags: superreview?(jst)
Attachment #245048 - Flags: review?(jst)
Comment on attachment 245048 [details] [diff] [review]
Patch to fix

r+sr=jst
Attachment #245048 - Flags: superreview?(jst)
Attachment #245048 - Flags: superreview+
Attachment #245048 - Flags: review?(jst)
Attachment #245048 - Flags: review+
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Flags: blocking1.9?
Resolution: --- → FIXED
*** Bug 360054 has been marked as a duplicate of this bug. ***
I can't actually reliably reduce the dataloss part even without the landed patch, so it's hard to tell what exactly failed with serializing the range.

It'd be great if someone else could verify that the dataloss issue has actually been fixed on the trunk and if so mark the bug VERIFIED
Component: DOM: Traversal-Range → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.