Bug 19377 - REGRESSION (r34109): "Inspect Element" on a node in a subframe doesn't highlight that node
Summary: REGRESSION (r34109): "Inspect Element" on a node in a subframe doesn't highli...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL: http://tivofaq.com/
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2008-06-03 09:39 PDT by Adam Roben (:aroben)
Modified: 2008-07-24 08:21 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (16.84 KB, patch)
2008-07-22 09:30 PDT, Timothy Hatcher
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2008-06-03 09:39:32 PDT
Choosing "Inspect Element" from the context menu on a node in a subframe doesn't highlight that node in the Elements panel.

To repro:
1. Go to http://tivofaq.com/
2. Right-click on the "ILoveMyTiVo" link at the top of the right frame
3. Choose "Inspect Element"

Results:
The Inspector appears, but the <frameset> element is highlighted, not the link you right-clicked on.
Comment 1 Adam Roben (:aroben) 2008-06-03 10:17:25 PDT
I think this was caused by r31890. What's happening is we're ending up with two JSInspectedObjectWrappers for the same JSHTMLHtmlElement. The two wrappers have different global objects. Presumably one of them is the subframe's global object and one of them is the main frame's. The effect of this is that the two wrappers fail a strict equality check (they'd fail a non-strict equality check, too).
Comment 2 Adam Roben (:aroben) 2008-06-03 10:21:34 PDT
(In reply to comment #1)
> Presumably one of them is the subframe's global
> object and one of them is the main frame's.

This guess turns out to be correct.
Comment 3 Adam Roben (:aroben) 2008-06-06 08:28:56 PDT
Actually, I suspect that r34109 was the cause of this bug. That change caused us to start caching wrappers per-global object. Prior to that we only had one wrapper for each JS object.
Comment 4 Timothy Hatcher 2008-07-22 09:30:53 PDT
Created attachment 22430 [details]
Proposed patch
Comment 5 Adam Roben (:aroben) 2008-07-22 10:11:15 PDT
Comment on attachment 22430 [details]
Proposed patch

"sameObjects" is a somewhat awkward name for an equality function. I think it needs some version of "to be" in it, like "areSameObject" or "objectsAreSame" or "objectsAreEqual". It also seems like we might as well call up to C++ to do the comparison on the unwrapped objects for real, if we're sending everything through a single bottleneck anyway.

r=me, but do consider renaming sameObjects.
Comment 6 Timothy Hatcher 2008-07-24 08:21:58 PDT
Landed in r35315.