WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
155497
[GTK] WebInspector broken after
r197620
https://bugs.webkit.org/show_bug.cgi?id=155497
Summary
[GTK] WebInspector broken after r197620
Alejandro G. Castro
Reported
2016-03-15 11:32:48 PDT
I get these errors when I try to open it: resource:///org/webkitgtk/inspector/UserInterface/Base/Setting.js:61:44: CONSOLE ERROR SecurityError: DOM Exception 18: An attempt was made to break through the security policy of the user agent. resource:///org/webkitgtk/inspector/UserInterface/Main.html:1:21: CONSOLE ERROR ReferenceError: Can't find variable: InspectorFrontendAPI resource:///org/webkitgtk/inspector/UserInterface/Main.html:1:21: CONSOLE ERROR ReferenceError: Can't find variable: InspectorFrontendAPI resource:///org/webkitgtk/inspector/UserInterface/Main.html:1:21: CONSOLE ERROR ReferenceError: Can't find variable: InspectorFrontendAPI resource:///org/webkitgtk/inspector/UserInterface/Main.html:1:21: CONSOLE ERROR ReferenceError: Can't find variable: InspectorFrontendAPI resource:///org/webkitgtk/inspector/UserInterface/Protocol/InspectorBackendCommands.js:32:17: CONSOLE ERROR ReferenceError: Can't find variable: InspectorBackend
Attachments
Patch
(1.73 KB, patch)
2016-03-17 06:16 PDT
,
Alejandro G. Castro
cgarcia
: commit-queue-
Details
Formatted Diff
Diff
Alternative patch
(8.19 KB, patch)
2016-03-17 06:56 PDT
,
Carlos Garcia Campos
pnormand
: review+
Details
Formatted Diff
Diff
Patch for landing.
(5.99 KB, patch)
2016-03-22 05:16 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-03-15 11:34:11 PDT
<
rdar://problem/25171910
>
Timothy Hatcher
Comment 2
2016-03-15 12:06:24 PDT
You likely need the same change that landed for Mac to fix this. See
bug 155265
.
http://trac.webkit.org/changeset/197920/trunk/Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm
Alejandro G. Castro
Comment 3
2016-03-17 06:16:57 PDT
Created
attachment 274289
[details]
Patch
Carlos Garcia Campos
Comment 4
2016-03-17 06:55:25 PDT
Comment on
attachment 274289
[details]
Patch This patch is correct, because we register resource scheme as local, but I think that was a mistake in the first place. A GResources is not like a local file, you can't replace by another file, for example, they are inmutable, always available and never fail to load. So, I think we should stop doing considering resource:// as local, which should also fix this problem.
Carlos Garcia Campos
Comment 5
2016-03-17 06:56:51 PDT
Created
attachment 274291
[details]
Alternative patch This is an alternative approach to stop considering resource:// as local.
Philippe Normand
Comment 6
2016-03-18 01:34:46 PDT
Comment on
attachment 274291
[details]
Alternative patch View in context:
https://bugs.webkit.org/attachment.cgi?id=274291&action=review
> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:833 > +bool WebInspectorProxy::isValidInspectorURL(const URL& url) > +{ > + return url.isLocalFile(); > +}
Shouldn't this rather use WebCore::SchemeRegistry::shouldTreatURLSchemeAsLocal() ?
Carlos Garcia Campos
Comment 7
2016-03-18 01:42:43 PDT
(In reply to
comment #6
)
> Comment on
attachment 274291
[details]
> Alternative patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=274291&action=review
> > > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:833 > > +bool WebInspectorProxy::isValidInspectorURL(const URL& url) > > +{ > > + return url.isLocalFile(); > > +} > > Shouldn't this rather use > WebCore::SchemeRegistry::shouldTreatURLSchemeAsLocal() ?
No, we changed that for GTK+ when we started to use GResources for the inspector, so now that we no longer consider GResources as local, we can bring back the previous code.
Alejandro G. Castro
Comment 8
2016-03-18 10:18:49 PDT
(In reply to
comment #4
)
> Comment on
attachment 274289
[details]
> Patch > > This patch is correct, because we register resource scheme as local, but I > think that was a mistake in the first place. A GResources is not like a > local file, you can't replace by another file, for example, they are > inmutable, always available and never fail to load. So, I think we should > stop doing considering resource:// as local, which should also fix this > problem.
I sounds like a better approach to me, thanks.
Philippe Normand
Comment 9
2016-03-21 05:04:17 PDT
Comment on
attachment 274291
[details]
Alternative patch Alright then, looks good to me!
Carlos Garcia Campos
Comment 10
2016-03-21 05:07:59 PDT
(In reply to
comment #9
)
> Comment on
attachment 274291
[details]
> Alternative patch > > Alright then, looks good to me!
Thanks! This requires WebKit2 owner approval, since I'm changing WebKit2 cross-platform code
Darin Adler
Comment 11
2016-03-21 12:09:50 PDT
Comment on
attachment 274291
[details]
Alternative patch View in context:
https://bugs.webkit.org/attachment.cgi?id=274291&action=review
Why add these functions just so we can make assertions? Lets just remove the assertions and not add the functions.
> Source/WebKit2/UIProcess/WebInspectorProxy.h:61 > +};
Semicolon here is incorrect.
Carlos Garcia Campos
Comment 12
2016-03-22 02:35:08 PDT
(In reply to
comment #11
)
> Comment on
attachment 274291
[details]
> Alternative patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=274291&action=review
> > Why add these functions just so we can make assertions? Lets just remove the > assertions and not add the functions.
There's also one actual check, and then the two assertions. But I think we can indeed get rid of it entirely, since then we are comparing the given URL with the platform provided main URL, so the initial check looks like an optimization that is never actually happening.
> > Source/WebKit2/UIProcess/WebInspectorProxy.h:61 > > +}; > > Semicolon here is incorrect.
Carlos Garcia Campos
Comment 13
2016-03-22 05:16:15 PDT
Created
attachment 274646
[details]
Patch for landing. Darin, please set cq+ if you are ok with this.
WebKit Commit Bot
Comment 14
2016-03-22 10:26:05 PDT
Comment on
attachment 274646
[details]
Patch for landing. Clearing flags on attachment: 274646 Committed
r198532
: <
http://trac.webkit.org/changeset/198532
>
WebKit Commit Bot
Comment 15
2016-03-22 10:26:12 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.
Top of Page
Format For Printing
XML
Clone This Bug