Closed Bug 610258 Opened 14 years ago Closed 13 years ago

Improve handling for stale elements in HTMLDocuments

Categories

(Testing Graveyard :: Mozmill, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: whimboo, Assigned: whimboo)

References

()

Details

(Whiteboard: [mozmill-1.5.2+])

Attachments

(1 file, 2 obsolete files)

Currently the handling of content elements is some-kind broken when it comes to page loads and reusing already created elements. In all the cases those elements cannot be accessed anymore because the document has been replaced (See the Google groups URL).

The source of the problem are the elementlib classes which all expect a document for the constructor call. This document reference is cached through their lifetime. While it works fine for elements in chrome scope, the document simply doesn't get replaced, in content we get those above problems.

We should investigate, even for the elements map rewrite Clint has proposed, if we couldn't work on the defaultView instead. That way we should always have valid element references because activeTab.document would retrieve the newly document. We should check how it would work with chrome and frames, but I don't expect any problem.

If we could find a simple solution I would even like to see it for 1.5.x.
I had an idea, tried it and hey, surprisingly it immediately worked for all types of elements we support. A test-run didn't show up any additional failures for all supported branches. Even one or two of the existing broken tests pass now.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #488776 - Flags: review?(ctalbert)
Because no interface changes are required I would like to ask if we can get it in the next minor release of Mozmill.
Whiteboard: [mozmill-2.0?] → [mozmill-1.5.2?]
Attachment #488776 - Flags: review?(ctalbert) → feedback?(ctalbert)
Comment on attachment 488776 [details] [diff] [review]
Patch v1 (no interface changes - doable for 1.5.x)

I think this will work.  I'm wondering if it works in all cases of chrome documents so I want to do some testing on it before we take it for 1.5.2, but as far as approach goes, I don't see anything terribly wrong with the idea.
Attachment #488776 - Flags: feedback?(ctalbert) → feedback+
As talked with Clint, I will not have time in the 1.5.2 train to work on a final patch with tests.
Assignee: hskupin → nobody
As we have a patch, and only requires some testing, I think we can get this in.  I'll take it for the time being, but it's prioritized after the JS <--> python work.
Assignee: nobody → ctalbert
Whiteboard: [mozmill-1.5.2?] → [mozmill-1.5.2+]
Assignee: ctalbert → jhammel
This patch is just Henrik's patch plus handling for the new elementslib.Selector() and a small testcase.
Attachment #488776 - Attachment is obsolete: true
Attachment #504888 - Flags: review?(ctalbert)
correct patch, whoops.
Attachment #504888 - Attachment is obsolete: true
Attachment #504891 - Flags: review?(ctalbert)
Attachment #504888 - Flags: review?(ctalbert)
Comment on attachment 504891 [details] [diff] [review]
Henrik's patch plus Selector handling and testcase

Looks great.  Thanks, Harth
Attachment #504891 - Flags: review?(ctalbert) → review+
Looks great. Verified fixed with the latest hotfix-1.5.2 pull.
Assignee: jhammel → hskupin
Status: RESOLVED → VERIFIED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: