699308 - [highlighter] highlighter closes during iframe requests

Status: VERIFIED FIXED

(DevTools :: General, defect, P1)

Description

[Rob Campbell [:rc] (:robcee)]

STR:

1. open a page with AJAX on it. (e.g., http://arstechnica.com/apple/news/2011/11/stale-mac-pro-lineup-has-pro-users-concerned.ars)
2. open the highlighter
3. wait.

Expected:

highligher stays up.

Actual:

When an XMLRPC request fires, the highlighter closes.

Comment 1

[Mihai Sucan [:msucan]]

This is caused by the InspectorProgressListener, see onStateChange():

    // If the request is about to happen in a new window, we are not concerned
    // about the request.
    if (aProgress.DOMWindow != this.IUI.win) {
      return;
    }

The check above is not sufficient.

Comment 2

[Mihai Sucan [:msucan]]

Attachment: proposed fix

This is the proposed fix. We just needed more fine-tuning of filtering the states. Please test and let me know if this patch fixes the problem. It does on my system.

The patch is missing a test because (1) it's late here and (2) we can't run mochitests anymore.

If we get a fix for running mochitests by tomorrow or Monday, I'd happily write a test for this patch.

Given the importance of this fix we should try as much as possible to land this in time for fx10.

Looking forward for your feedback. Thank you!

Comment 3

[Rob Campbell [:rc] (:robcee)]

tests should be fixed now if you repull from m-c or fx-team.

I'll test this locally and give it a read through. Thanks!

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 572216 [details] [diff] [review]
proposed fix

This tested well here. I ran it against Yahoo's front page and moused over the middle part of the page that has the different stories associated with pictures and the Highlighter stayed up. I'll look around on a couple of other sites I was seeing this happening on, but I think this'll work.

Comment 5

[Mihai Sucan [:msucan]]

Attachment: fix + test

Updated patch. Added a mochitest. Two javascript:location.reload() calls in the iframe are sufficient to trigger the bug.

Looking forward for your review. Thank you!

(asking both for review, I don't know who has time to get to review the patch sooner)

Comment 6

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 572293 [details] [diff] [review]
fix + test

yup!

Comment 7

[Dave Camp (:dcamp)]

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

Comment 8

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/ec6b4d73b906

Comment 9

[Teodosia Pop]

Verified fixed using
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a2) Gecko/20111124 Firefox/10.0a2
and
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111123 Firefox/10.0a2