Bug 23502

Summary:

To isolate code accessing the inspected DOM in the facade layer

Product:

WebKit

Reporter:

Sergey Ryazanov <serya>

Component:

Web Inspector (Deprecated)

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Enhancement

CC:

dglazkov, ojan, serya, timothy

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Implementation of the facade layer for the inspected DOM.	none
The default implementation of the facade layer	timothy: review-
Added WebCore.vcproj	timothy: review-
Screenshot of bug	none
A screenshot with additional style rules	none
Fixed the issue with styles	timothy: review-
Image showing the new issue	none
Fixed issue with striked out styles	timothy: review-
Console bug	none
Fixed the code displaying results in the Console	none

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	Details Formatted Diff Diff
The default implementation of the facade layer (106.15 KB, patch) 2009-02-02 03:41 PST, Sergey Ryazanov	timothy: review-	Details Formatted Diff Diff
Added WebCore.vcproj (116.25 KB, patch) 2009-02-06 02:27 PST, Sergey Ryazanov	timothy: review-	Details Formatted Diff Diff
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-	Details Formatted Diff Diff
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-	Details Formatted Diff Diff
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	Details Formatted Diff Diff
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.