RESOLVED WONTFIX 23502
To isolate code accessing the inspected DOM in the facade layer 
https://bugs.webkit.org/show_bug.cgi?id=23502
Summary To isolate code accessing the inspected DOM in the facade layer
Sergey Ryazanov
Reported 2009-01-23 05:48:47 PST
Currently the Inspector code directly manipulates either its own UI DOM nodes and DOM nodes of the inspected page. To simplify localization the code which access the inspected DOM it is suggested to isolate it in the facade layer. It will help in implementation of out-of-process DOM access in Chromium.
Attachments
Implementation of the facade layer for the inspected DOM. (102.65 KB, patch)
2009-01-23 06:01 PST, Sergey Ryazanov 		no flags
DetailsFormatted DiffDiff
The default implementation of the facade layer (106.15 KB, patch)
2009-02-02 03:41 PST, Sergey Ryazanov 		timothy: review-
DetailsFormatted DiffDiff
Added WebCore.vcproj (116.25 KB, patch)
2009-02-06 02:27 PST, Sergey Ryazanov 		timothy: review-
DetailsFormatted DiffDiff
Screenshot of bug (21.75 KB, image/png)
2009-02-13 15:54 PST, Timothy Hatcher 		no flags
Details
A screenshot with additional style rules (164.43 KB, image/png)
2009-02-18 10:23 PST, Sergey Ryazanov 		no flags
Details
Fixed the issue with styles (118.27 KB, patch)
2009-02-18 14:29 PST, Sergey Ryazanov 		timothy: review-
DetailsFormatted DiffDiff
Image showing the new issue (54.29 KB, image/png)
2009-02-27 11:01 PST, Timothy Hatcher 		no flags
Details
Fixed issue with striked out styles (118.98 KB, patch)
2009-03-10 11:48 PDT, Sergey Ryazanov 		timothy: review-
DetailsFormatted DiffDiff
Console bug (5.44 KB, image/png)
2009-03-13 13:58 PDT, Timothy Hatcher 		no flags
Details
Fixed the code displaying results in the Console (120.42 KB, patch)
2009-03-15 07:43 PDT, Sergey Ryazanov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.
Sergey Ryazanov
Comment 1 2009-01-23 06:01:23 PST
Created attachment 26968 [details] Implementation of the facade layer for the inspected DOM. InspectedWindowFacade.js makes up the interface for the inspected DOM.
Sergey Ryazanov
Comment 2 2009-02-02 03:41:37 PST
Created attachment 27244 [details] The default implementation of the facade layer Resolved conflicts with new changes and fixed ChangeLog
Timothy Hatcher
Comment 3 2009-02-03 13:48:52 PST
Comment on attachment 27244 [details] The default implementation of the facade layer The code is fine, so this is ready to land except the InspectedWindowFacade.js file needs added to the WebCore.vcproj. The patch also doesn't see to have the ChangeLog like you mentioned.
Sergey Ryazanov
Comment 4 2009-02-06 02:27:42 PST
Created attachment 27384 [details] Added WebCore.vcproj
Timothy Hatcher
Comment 5 2009-02-13 15:54:08 PST
Testing out this patch before I landed it, I noticed that no styles are showing up in the Elements panel. Only some computed style properties are showing.
Timothy Hatcher
Comment 6 2009-02-13 15:54:39 PST
Created attachment 27672 [details] Screenshot of bug
Sergey Ryazanov
Comment 7 2009-02-18 10:23:48 PST
Created attachment 27755 [details] A screenshot with additional style rules Can't reproduce the issue. Here is a example (I made the "Computed Style" lable in capital case to be sure I see patched version). Currently I see no difference with the code without patch in this aspect.
Sergey Ryazanov
Comment 8 2009-02-18 14:29:28 PST
Created attachment 27763 [details] Fixed the issue with styles Style rules returned by getMatchedCSSRules was not included int the pane (this fixed here). The computed styles, styls from attributes and inline styles was not toughed (have been working).
Dimitri Glazkov (Google)
Comment 9 2009-02-26 09:41:29 PST
Do we need corresponding XCode proj changes?
Timothy Hatcher
Comment 10 2009-02-26 10:48:48 PST
(In reply to comment #9) > Do we need corresponding XCode proj changes? Nothing in the Xcode project needs to change since it copies the whole inspector folder.
Timothy Hatcher
Comment 11 2009-02-27 11:01:17 PST
Comment on attachment 27763 [details] Fixed the issue with styles There is still an issue with the Styles pane, all the items are "strike out" and computed style only has width, height and display properties.
Timothy Hatcher
Comment 12 2009-02-27 11:01:37 PST
Created attachment 28085 [details] Image showing the new issue
Dimitri Glazkov (Google)
Comment 13 2009-02-27 13:02:18 PST
Dooh! Almost nearly landed this one.
Sergey Ryazanov
Comment 14 2009-03-10 11:48:45 PDT
Created attachment 28444 [details] Fixed issue with striked out styles
Timothy Hatcher
Comment 15 2009-03-11 15:27:28 PDT
I will try this out and review it. What was the main area of the patch that changed? What was the bug?
Sergey Ryazanov
Comment 16 2009-03-12 02:02:12 PDT
(In reply to comment #15) > I will try this out and review it. What was the main area of the patch that > changed? What was the bug? The bug was in the method WebInspector.StylesSidebarPane.update: the loop "for (var j = 0; j < style.length; ++j)" was never iterated since style.length was undefined in the facade layer.
Timothy Hatcher
Comment 17 2009-03-13 13:57:52 PDT
Comment on attachment 28444 [details] Fixed issue with striked out styles Styles seems to be working fine now. Thanks! But it looks like the console is broken. I am not seeing output for common expressions. See attached screenshot.
Timothy Hatcher
Comment 18 2009-03-13 13:58:40 PDT
Created attachment 28596 [details] Console bug
Timothy Hatcher
Comment 19 2009-03-13 13:59:59 PDT
Maybe it would be best to break this up into smaller patches so the parts that are working can be landed now, then convert more and more.
Sergey Ryazanov
Comment 20 2009-03-15 07:43:44 PDT
Created attachment 28635 [details] Fixed the code displaying results in the Console Fixed methods _formatnode and _formatobject. Also fixed bug in the Scripts panel (incorrect using of the extraProperties argument).
Sergey Ryazanov
Comment 21 2009-03-16 10:37:50 PDT
We decided to simplify the changes.
Maciej Stachowiak
Comment 22 2009-05-21 22:02:47 PDT
Reopening, since this still has a patch for review.
Timothy Hatcher
Comment 23 2009-05-21 22:09:37 PDT
They decided to change how this was done and filed other bugs.
Note You need to log in before you can comment on or make changes to this bug.