RESOLVED FIXED Bug 130343
REGRESSION(r165704): [GTK] Inspector resources not correctly generated
https://bugs.webkit.org/show_bug.cgi?id=130343
Summary REGRESSION(r165704): [GTK] Inspector resources not correctly generated
Carlos Garcia Campos
Reported 2014-03-17 09:08:49 PDT
Since r165704 there are two resources that are now generated. This revealed some problems in the way we generate the inspector resources: - The script generate-inspector-gresource-manifest.py doesn't receive the list of resources from the makefile, but it duplicates the expression in the makefile to generate the resources list again. This is very easy to end up out of sync, and people from other ports are not expected to know we have this duplication. - We are generating the resources xml file using paths relative to Source/WebInspectorUI, but the new files are in DerivedSources. We should use top source dir and use full paths Source/ buldir/DerivedSource, etc. To keep the same names for the gresources, we need to use alias, something like: <file alias="UserInterface/External/CodeMirror/css.js">Source/WebInspectorU/UserInterface/External/CodeMirror/css.js</file>
Attachments
Patch (20.48 KB, patch)
2014-03-18 19:34 PDT, Martin Robinson
cgarcia: review-
Updated patch (16.42 KB, patch)
2014-04-02 06:00 PDT, Carlos Garcia Campos
no flags
Martin Robinson
Comment 1 2014-03-18 19:25:24 PDT
This is a good moment to fix inspector resource generation in general, by moving it to WebCore.
Martin Robinson
Comment 2 2014-03-18 19:34:54 PDT
Sergio Villar Senin
Comment 3 2014-03-20 04:58:09 PDT
(In reply to comment #2) > Created an attachment (id=227146) [details] > Patch The inspector is shown for me with this patch, but the resources tab shows nothing.
Martin Robinson
Comment 4 2014-03-20 11:18:03 PDT
(In reply to comment #3) > (In reply to comment #2) > > Created an attachment (id=227146) [details] [details] > > Patch > > The inspector is shown for me with this patch, but the resources tab shows nothing. I'm not sure it's related to this patch. I get errors like these in WebKit1: ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @626: IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @86: 0 ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Protocol/InspectorBackend.js @259: Uncaught exception in inspector page: ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @626: IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @86: 0 ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @626: IndexSizeError: DOM Exception 1: Index or size was negative, or greater than the allowed value. ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Base/ImageUtilities.js @86: 0 ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Views/ResourceSidebarPanel.js @123: TypeError: null is not an object (evaluating 'contentView.showSourceCode') ** Message: console message: resource:///org/webkitgtk/inspector/UserInterface/Views/ResourceSidebarPanel.js @116: TypeError: null is not an object (evaluating 'contentView.showDOMTree')
Carlos Garcia Campos
Comment 5 2014-03-21 01:31:11 PDT
(In reply to comment #3) > (In reply to comment #2) > > Created an attachment (id=227146) [details] [details] > > Patch > > The inspector is shown for me with this patch, but the resources tab shows nothing. This happened to me as well, but then I recompiled to debug it and started to work. It haven't failed again.
Carlos Garcia Campos
Comment 6 2014-03-21 01:31:44 PDT
it *hasn't* :-P
Carlos Garcia Campos
Comment 7 2014-03-21 02:29:38 PDT
Comment on attachment 227146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227146&action=review I guess we need to update the autotools build too > Tools/gtk/generate-inspector-gresource-manifest.py:52 > + gresources_file_content = \ > + """<?xml version=1.0 encoding=UTF-8?> > + <gresources> > + <gresource prefix="/org/webkitgtk/inspector"> > """ Now that we have a file open for writing already at this point, maybe we can write to the file, instead of buffering the whole file in memory and writing to the file in one shoot.
Carlos Garcia Campos
Comment 8 2014-03-21 02:41:16 PDT
Manuel Rego Casasnovas
Comment 9 2014-03-28 16:19:10 PDT
(In reply to comment #3) > (In reply to comment #2) > > Created an attachment (id=227146) [details] [details] > > Patch > > The inspector is shown for me with this patch, but the resources tab shows nothing. The inspector appears here too, but it's not working either.
Manuel Rego Casasnovas
Comment 10 2014-03-31 14:43:58 PDT
(In reply to comment #9) > (In reply to comment #3) > > (In reply to comment #2) > > > Created an attachment (id=227146) [details] [details] [details] > > > Patch > > > > The inspector is shown for me with this patch, but the resources tab shows nothing. > > The inspector appears here too, but it's not working either. After a clean build the inspector is working again for me with this patch.
Carlos Garcia Campos
Comment 11 2014-04-01 00:16:12 PDT
Comment on attachment 227146 [details] Patch Patch looks good to me in general, but please, don't compile the resources in WebCore, but in libwebkit and libwebkit2, even if there are some rules duplicated in cmake files we are going to remove the wk1 ones soon, so it's not a problem. Also consider using the file descriptor instead of buffering the whole file.
Carlos Garcia Campos
Comment 12 2014-04-02 06:00:22 PDT
Created attachment 228388 [details] Updated patch I've updated the patch, the inspector has been broken for long time and it's high priority now.
Gustavo Noronha (kov)
Comment 13 2014-04-02 07:03:28 PDT
Comment on attachment 228388 [details] Updated patch OK, but looks like efl-wk2 will need a fix
Carlos Garcia Campos
Comment 14 2014-04-02 08:10:02 PDT
Comment on attachment 228388 [details] Updated patch EFL failure looks unrelated to me
WebKit Commit Bot
Comment 15 2014-04-02 08:39:06 PDT
Comment on attachment 228388 [details] Updated patch Clearing flags on attachment: 228388 Committed r166648: <http://trac.webkit.org/changeset/166648>
WebKit Commit Bot
Comment 16 2014-04-02 08:39:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.