[GTK] Remote inspector server should send an error page if resources not found
Created attachment 203568 [details] Patch
Comment on attachment 203568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203568&action=review > Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59 > + if (error->code == G_IO_ERROR_NOT_FOUND) { I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that.
Comment on attachment 203568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203568&action=review Have you considered doing a g_warning on all of the return false cases? That should make it much easier to debug things, generally. >> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59 >> + if (error->code == G_IO_ERROR_NOT_FOUND) { > > I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that. I think that makes sense, yeah. On second thought, this is likely to hit technical people, so maybe just don't do the error->code check: show it and error->message as well, should make it easier for those people to debug it. > Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:63 > + builder.appendLiteral(", make sure that you have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder.</body></html>"); How about "make sure you ran make install or have set WEBKIT_…"
Comment on attachment 203568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203568&action=review >>> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:59 >>> + if (error->code == G_IO_ERROR_NOT_FOUND) { >> >> I wonder if I should send some basic error page for other errors - unknown error during retrieving inspector resources - or something like that. > > I think that makes sense, yeah. On second thought, this is likely to hit technical people, so maybe just don't do the error->code check: show it and error->message as well, should make it easier for those people to debug it. yep, makes sense. Something like "Error (Code: %d, Message: %s) occurred during fetching webinspector resource files. Make sure you ran make install or have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder"? >> Source/WebKit2/UIProcess/InspectorServer/gtk/WebInspectorServerGtk.cpp:63 >> + builder.appendLiteral(", make sure that you have set WEBKIT_INSPECTOR_SERVER_PATH in your environment to point to webinspector folder.</body></html>"); > > How about "make sure you ran make install or have set WEBKIT_…" ok
I can add also LOG(InspectorServer, ... . It should be consistent with other remote inspector code (see for example in ./Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:77: LOG(InspectorServer, "To start inspector server set WEBKIT_INSPECTOR_SERVER to 127.0.0.1:2999 for example.");
Using LOG sounds good too, thing is it will only show up when running on a debug build with WEBKIT_DEBUG set to InspectorServer, so we should not rely on it as the main means of letting the user know there was a problem.
(In reply to comment #6) > Using LOG sounds good too, thing is it will only show up when running on a debug build with WEBKIT_DEBUG set to InspectorServer, so we should not rely on it as the main means of letting the user know there was a problem. ok, I'll make change with g_warning.
Created attachment 206455 [details] Patch
Comment on attachment 206455 [details] Patch Attachment 206455 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1052298 New failing tests: svg/batik/filters/feTile.svg
Created attachment 206466 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Comment on attachment 206455 [details] Patch Clearing flags on attachment: 206455 Committed r152633: <http://trac.webkit.org/changeset/152633>
All reviewed patches have been landed. Closing bug.