This will include all the items in "Develop > User Agent" (including a custom input).
<rdar://problem/47359292>
Created attachment 360225 [details] Patch
Created attachment 360226 [details] [Image] After Patch is applied (Popover)
Created attachment 360227 [details] [Image] After Patch is applied (<select>)
Created attachment 360228 [details] [Image] After Patch is applied (Custom)
This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Comment on attachment 360225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360225&action=review r=me but I think you should be able to add a test: function printUserAgent() { TestPage.addResult(`UserAgent: ${navigator.userAgent}`); } ... async test() { await InspectorTest.evaluate(`printUserAgent()`); InspectorTest.log("Overriding..."); await PageAgent.overrideUserAgent("test user agent"); await InspectorTest.reloadPage(); await InspectorTest.evaluate(`printUserAgent()`); InspectorTest.log("Restoring..."); await PageAgent.overrideUserAgent(); await InspectorTest.reloadPage(); await InspectorTest.evaluate(`printUserAgent()`); } > Source/WebCore/inspector/agents/InspectorPageAgent.cpp:361 > + m_userAgentOverride = value ? *value : String(); While I like `String()` some people have been doing `{ }`. I'll leave that up to you. > Source/WebInspectorUI/UserInterface/Base/Main.js:1966 > + if (value === WI._overridenDeviceSettings) > + return; This looks wrong. Looks like it should be be comparing the user agent, not against the Set: if (value === this._overriddenDeviceUserAgent) return; > Source/WebInspectorUI/UserInterface/Base/Main.js:2093 > + ], > + [ Style: You could put these on the same line to save some space. > Source/WebInspectorUI/UserInterface/Base/Main.js:2101 > + { name: `Internet Explorer 8`, value: "Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.0; Trident/4.0)" }, > + { name: `Internet Explorer 7`, value: "Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0)" }, I don't think we need so many of these. I realize Safari has them... > Source/WebInspectorUI/UserInterface/Base/Main.js:2119 > + // FIXME: add separators between groups If you add a <hr> as a child of a <select> it will create a separator. AFAIK this is non-standard but WebKit has supported that forever. So you could do: if (group !== userAgents[0]) userAgentSelect.appendChild(document.createElement("hr")); > Source/WebInspectorUI/UserInterface/Base/Main.js:2144 > + if (inputEvent.keyCode === WI.KeyboardShortcut.Key.Enter.keyCode) > + applyOverriddenUserAgent(userAgentInput.value, true); Do we really want to do this on keydown? How about onchange or oninput? > Source/WebInspectorUI/UserInterface/Base/Main.js:2163 > + userAgentInput.selectionStart = userAgentInput.selectionEnd = 0; > + userAgentInput.focus(); Interesting, why not just `userAgentInput.select()`?
I see rniwa just landed userAgentForJavaScript. You might need to move this code a bit.
Comment on attachment 360225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360225&action=review (In reply to Joseph Pecoraro from comment #7) > r=me but I think you should be able to add a test: > > function printUserAgent() { > TestPage.addResult(`UserAgent: ${navigator.userAgent}`); > } > ... > async test() { > await InspectorTest.evaluate(`printUserAgent()`); > InspectorTest.log("Overriding..."); > await PageAgent.overrideUserAgent("test user agent"); > await InspectorTest.reloadPage(); > await InspectorTest.evaluate(`printUserAgent()`); > InspectorTest.log("Restoring..."); > await PageAgent.overrideUserAgent(); > await InspectorTest.reloadPage(); > await InspectorTest.evaluate(`printUserAgent()`); > } I actually tried exactly doing that (almost identical to what you wrote), but after the first reload there would be exceptions that `TestPage` wasn't defined. >> Source/WebInspectorUI/UserInterface/Base/Main.js:1966 >> + return; > > This looks wrong. Looks like it should be be comparing the user agent, not against the Set: > > if (value === this._overriddenDeviceUserAgent) > return; Oops. >> Source/WebInspectorUI/UserInterface/Base/Main.js:2093 >> + [ > > Style: You could put these on the same line to save some space. I personally hate that style :| >> Source/WebInspectorUI/UserInterface/Base/Main.js:2119 >> + // FIXME: add separators between groups > > If you add a <hr> as a child of a <select> it will create a separator. AFAIK this is non-standard but WebKit has supported that forever. > > So you could do: > > if (group !== userAgents[0]) > userAgentSelect.appendChild(document.createElement("hr")); I spent so long trying to find an alternative T.T I even almost went and added something to HTMLOptionElement to specifically support this. >> Source/WebInspectorUI/UserInterface/Base/Main.js:2144 >> + applyOverriddenUserAgent(userAgentInput.value, true); > > Do we really want to do this on keydown? How about onchange or oninput? I don't think we want to apply it on every input, as we reload the page. I only want it to happen when the user is "done". >> Source/WebInspectorUI/UserInterface/Base/Main.js:2163 >> + userAgentInput.focus(); > > Interesting, why not just `userAgentInput.select()`? `.select()` causes the <input> to scroll its contents to the end. I thought that looked bad. This way, you see the beginning of the custom value, and can scroll/key over to see the rest of it.
Created attachment 360246 [details] Patch
Comment on attachment 360246 [details] Patch Clearing flags on attachment: 360246 Committed r240549: <https://trac.webkit.org/changeset/240549>
All reviewed patches have been landed. Closing bug.