Bug 151075 - Web Inspector: Provide a way to override the WKWebView remote inspector name
Summary: Web Inspector: Provide a way to override the WKWebView remote inspector name
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Other
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-11-09 19:50 PST by Joseph Pecoraro
Modified: 2015-11-13 15:47 PST (History)
10 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (11.33 KB, patch)
2015-11-09 19:52 PST, Joseph Pecoraro 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-11-09 19:50:09 PST 
* SUMMARY
Provide a way to override the WKWebView remote inspector name.
Comment 1 Joseph Pecoraro 2015-11-09 19:50:35 PST 
<rdar://problem/23475086>
Comment 2 Joseph Pecoraro 2015-11-09 19:52:08 PST 
Created attachment 265140 [details]
[PATCH] Proposed Fix
Comment 3 WebKit Commit Bot 2015-11-09 19:54:59 PST 
Attachment 265140 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:314:  The parameter name "name" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/page/PageDebuggable.h:57:  The parameter name "name" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 BJ Burg 2015-11-10 08:50:49 PST 
Comment on attachment 265140 [details]
[PATCH] Proposed Fix

Is name reported as the Debuggable's title? I don't see a usage at the WK2 or RemoteInspector side. Otherwise, looks fine to me.
Comment 5 Joseph Pecoraro 2015-11-10 10:29:36 PST 
Comment on attachment 265140 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=265140&action=review

> Source/WebCore/page/PageDebuggable.cpp:48
>  String PageDebuggable::name() const

This is the use on the RemoteInspector side. When pushing a listing we get the name() of each debuggable.
Comment 6 Anders Carlsson 2015-11-10 10:44:40 PST 
Wouldn't it be better to make this a configuration parameter instead?
Comment 7 BJ Burg 2015-11-10 11:50:12 PST 
Comment on attachment 265140 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=265140&action=review

>> Source/WebCore/page/PageDebuggable.cpp:48
>>  String PageDebuggable::name() const
> 
> This is the use on the RemoteInspector side. When pushing a listing we get the name() of each debuggable.

Oh, missed that. Thanks.
Comment 8 BJ Burg 2015-11-10 11:52:49 PST 
(In reply to comment #6)
> Wouldn't it be better to make this a configuration parameter instead?

That would work if we know what to name the view and it will never change. But, I think the idea of this patch is to override crappy page names as the pages are loaded into the WKWebView. Joe?
Comment 9 Tim Horton 2015-11-10 11:56:07 PST 
(In reply to comment #8)
> (In reply to comment #6)
> > Wouldn't it be better to make this a configuration parameter instead?
> 
> That would work if we know what to name the view and it will never change.
> But, I think the idea of this patch is to override crappy page names as the
> pages are loaded into the WKWebView. Joe?

Yeah, I think it's very feasible that clients will want to change the name dynamically.
Comment 10 Joseph Pecoraro 2015-11-10 17:40:00 PST 
Are there any open questions?
Comment 11 Joseph Pecoraro 2015-11-13 11:46:51 PST 
<http://trac.webkit.org/changeset/192437>
Comment 12 Joseph Pecoraro 2015-11-13 15:47:31 PST 
Follow-up to add Availability Macros:
<http://trac.webkit.org/changeset/192446>