WebView’s -_setFixedLayoutSize: and a few other methods allow clients such as UIKit to control the viewport parameters in a way that overrides anything done by the webpage. Clients that use WKWebView would like to have similar control. However, WKWebView’s _fixedLayoutSize property does not have the same effect.
<rdar://problem/30331795>
Created attachment 367094 [details] Patch
Comment on attachment 367094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367094&action=review > Source/WebCore/dom/Document.h:414 > + ViewportArguments viewportArguments() const { return m_overrideViewportArguments ? *m_overrideViewportArguments : m_viewportArguments; } valueOr? Or is it value_or()? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:494 > + [viewportArgumentPairs enumerateKeysAndObjectsUsingBlock:makeBlockPtr([&] (NSString *key, NSString *value, BOOL* stop) { > + String keyString = key; > + String valueString = value; > + WebCore::setViewportFeature(viewportArguments, keyString, valueString, viewportFitEnabled, [] (WebCore::ViewportErrorCode, const String& errorMessage) { > + NSLog(@"-[WKWebView _setOverrideViewportArguments]: Error parsing viewport argument: %s", errorMessage.utf8().data()); > + }); > + }).get()]; It seems like an easy mistake to make with this SPI would be to use non-string values: @{ @"width" : @1000 }. What's going to happen there? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:511 > + _page->setOverrideViewportArguments(viewportArgumentsFromDictionary(viewportArguments, _page->preferences().viewportFitEnabled())); If preferences().viewportFitEnabled() changes after we've pushed these viewport arguments, they will not adjust, right? Should they? Actually, can viewportArguments contain @"viewport-fit"?
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 367094 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367094&action=review > > > Source/WebCore/dom/Document.h:414 > > + ViewportArguments viewportArguments() const { return m_overrideViewportArguments ? *m_overrideViewportArguments : m_viewportArguments; } > > valueOr? Or is it value_or()? Sure! > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:494 > > + [viewportArgumentPairs enumerateKeysAndObjectsUsingBlock:makeBlockPtr([&] (NSString *key, NSString *value, BOOL* stop) { > > + String keyString = key; > > + String valueString = value; > > + WebCore::setViewportFeature(viewportArguments, keyString, valueString, viewportFitEnabled, [] (WebCore::ViewportErrorCode, const String& errorMessage) { > > + NSLog(@"-[WKWebView _setOverrideViewportArguments]: Error parsing viewport argument: %s", errorMessage.utf8().data()); > > + }); > > + }).get()]; > > It seems like an easy mistake to make with this SPI would be to use > non-string values: @{ @"width" : @1000 }. What's going to happen there? A fair point! We should probably throw an exception. > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:511 > > + _page->setOverrideViewportArguments(viewportArgumentsFromDictionary(viewportArguments, _page->preferences().viewportFitEnabled())); > > If preferences().viewportFitEnabled() changes after we've pushed these > viewport arguments, they will not adjust, right? Correct. > Should they? Probably? Though I bet we have this problem with <meta viewport> too. And API clients can just do things in the right order :( but it's true, it would be nice if it worked. > Actually, can viewportArguments contain @"viewport-fit"? Yes! (if viewportFitEnabled is YES)
(In reply to Tim Horton from comment #4) > (In reply to Simon Fraser (smfr) from comment #3) > > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:511 > > > + _page->setOverrideViewportArguments(viewportArgumentsFromDictionary(viewportArguments, _page->preferences().viewportFitEnabled())); > > > > If preferences().viewportFitEnabled() changes after we've pushed these > > viewport arguments, they will not adjust, right? > > Correct. > > > Should they? > > Probably? Though I bet we have this problem with <meta viewport> too. And > API clients can just do things in the right order :( but it's true, it would > be nice if it worked. > > > Actually, can viewportArguments contain @"viewport-fit"? > > Yes! (if viewportFitEnabled is YES) Oh, viewportFitEnabled is not affected by the value of @"viewport-fit", OK.
Created attachment 367103 [details] Patch
> > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:511 > > > + _page->setOverrideViewportArguments(viewportArgumentsFromDictionary(viewportArguments, _page->preferences().viewportFitEnabled())); > > > > If preferences().viewportFitEnabled() changes after we've pushed these > > viewport arguments, they will not adjust, right? > > Correct. > > > Should they? > > Probably? Though I bet we have this problem with <meta viewport> too. And > API clients can just do things in the right order :( but it's true, it would > be nice if it worked. I'm not sure how to easily get this right, and it doesn't matter while this is SPI; I fixed all the other things.
Comment on attachment 367103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367103&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:508 > + _overrideViewportArguments = adoptNS([viewportArguments copy]); Should we deep-copy all the strings? They may be mutable and mutated by the caller after this.
(In reply to Simon Fraser (smfr) from comment #8) > Comment on attachment 367103 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367103&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:508 > > + _overrideViewportArguments = adoptNS([viewportArguments copy]); > > Should we deep-copy all the strings? They may be mutable and mutated by the > caller after this. We could also make it write-only like some of the other overrides, and not even store the dictionary at all (just the parsed ViewportArguments)
Created attachment 367146 [details] Patch
Created attachment 367153 [details] Patch
Comment on attachment 367153 [details] Patch Clearing flags on attachment: 367153 Committed r244151: <https://trac.webkit.org/changeset/244151>
All reviewed patches have been landed. Closing bug.
New API test TestWebKitAPI.WebKit.OverrideViewportArguments added in https://trac.webkit.org/changeset/244151 is failing on iOS Simulator Debug https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29/builds/3239/steps/run-api-tests/logs/stdio TestWebKitAPI.WebKit.OverrideViewportArguments 2019-04-10 17:41:45.743 TestWebKitAPI[3470:6770579] -[WKWebView _overrideViewportWithArguments:]: Error parsing viewport argument: Viewport argument key "garbage" not recognized and ignored. LEAK: 1 WebProcessPool LEAK: 1 WebPageProxy /Volumes/Data/slave/ios-simulator-12-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/OverrideViewportArguments.mm:54 Expected equality of these values: 1. Which is: 1 [webView scrollView].zoomScale Which is: 0.25
Any progress on this failure? It is failing every run on the bots.
(In reply to Shawn Roberts from comment #15) > Any progress on this failure? It is failing every run on the bots. The test passes 100% of the time when run locally :(
(In reply to Tim Horton from comment #16) > (In reply to Shawn Roberts from comment #15) > > Any progress on this failure? It is failing every run on the bots. > > The test passes 100% of the time when run locally :( Ah! It fails in debug. Will take a peek.