610258 - Improve handling for stale elements in HTMLDocuments

Status: VERIFIED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v1 (no interface changes - doable 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.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Because no interface changes are required I would like to ask if we can get it in the next minor release of Mozmill.

Comment 3

[cmtalbert]

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.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As talked with Clint, I will not have time in the 1.5.2 train to work on a final patch with tests.

Comment 5

[cmtalbert]

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.

Comment 6

[Heather Arthur [:harth]]

Attachment: Henrik's patch plus Selector handling and testcase

This patch is just Henrik's patch plus handling for the new elementslib.Selector() and a small testcase.

Comment 7

[Heather Arthur [:harth]]

Attachment: Henrik's patch plus Selector handling and testcase

correct patch, whoops.

Comment 8

[cmtalbert]

Comment on attachment 504891 [details] [diff] [review]
Henrik's patch plus Selector handling and testcase

Looks great.  Thanks, Harth

Comment 9

[Heather Arthur [:harth]]

1.5.2:
https://github.com/mozautomation/mozmill/commit/3f14c7c66a6479e476855a613c32bdb62a4e1615

master:
https://github.com/mozautomation/mozmill/commit/0d5506297f1ca84f6d3ed4443bfebeadca5de493

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Looks great. Verified fixed with the latest hotfix-1.5.2 pull.