Regression(255659@main) Unable to log into twitch.tv #10760

cdumez · 2023-02-27T22:26:13Z

`541ca5a`

Regression(255659@main) Unable to log into twitch.tv
https://bugs.webkit.org/show_bug.cgi?id=253026
rdar://105891522

Reviewed by Wenson Hsieh and Darin Adler.

The Twitch.tv login was failing because `screen.orientation` was returning
"portrait-primary" on macOS. The natural/default orientation on desktop should
be "landscape-primary" so this was confusing Twitch.

Also update our `screen.angle` logic to take into account the natural
orientation based on:
- https://w3c.github.io/screen-orientation/#dfn-screen-orientation-values-table

I have verified that Chrome on macOS returns "landscape-primary" for the type
and 0 for the angle. Our behavior is now aligned.

* LayoutTests/fast/screen-orientation/natural-orientation-expected.txt: Added.
* LayoutTests/fast/screen-orientation/natural-orientation.html: Added.
* LayoutTests/platform/ios/screen-orientation/natural-orientation-expected.txt: Added.
* Source/WebCore/page/ScreenOrientation.cpp:
(WebCore::ScreenOrientation::lock):
* Source/WebCore/page/ScreenOrientationType.h:
(WebCore::naturalScreenOrientationType):
* Source/WebCore/platform/ScreenOrientationProvider.cpp:
(WebCore::ScreenOrientationProvider::currentOrientation):
* Source/WebKit/WebProcess/WebCoreSupport/WebScreenOrientationManager.cpp:
(WebKit::WebScreenOrientationManager::currentOrientation):

Canonical link: https://commits.webkit.org/260944@main

491d27d

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🛠 gtk~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 api-gtk~~
	~~🛠 tv~~	~~🧪 mac-wk2~~
	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~
	~~🛠 watch~~	~~🧪 mac-wk2-stress~~
	❌ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

webkit-early-warning-system · 2023-02-27T22:26:55Z

EWS run on previous version of this PR (hash 2b671ee)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	❌ 🧪 gtk-wk2
	~~🧪 api-ios~~	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

darinadler · 2023-02-28T00:17:17Z

Source/WebCore/page/ScreenOrientationType.h

+#if PLATFORM(IOS_FAMILY)
+    return ScreenOrientationType::PortraitPrimary;
+#else
+    // On Desktop, the natural orientation must be landscape-primary.
+    return ScreenOrientationType::LandscapePrimary;
+#endif


For Apple platforms there are the following: iPhone, iPad, watchOS, tvOS, macOS. To me, it is obvious that iPhone is clearly portrait-primary. Not as clear that iPad, watchOS, and tvOS all should be.

We had this discussion a while back with Tim and Wenson. iPad apparently doesn't have a clear "natural" orientation so we had decided to stick with PortraitPrimary like iPhone.

For watchOS and tvOS, we didn't explicitly discuss it so I had lumped them all together. I don't think the screen orientation API is super relevant to these platforms. We may want to refine this at some point though.

For now at least, this patch is not changing behavior for these platforms. The behavior change is for macOS at this point, where we have a clear regression.

And presumably also changes behavior for all non-Apple platforms?

If you wrote PLATFORM(IOS) it would be iPhone and iPad; that’s what I would do.

Other platforms don't support this yet:

bool defaultShouldEnableScreenOrientationAPI() { #if PLATFORM(MAC) return true; #elif PLATFORM(IOS_FAMILY) static bool shouldEnableScreenOrientationAPI = linkedOnOrAfterSDKWithBehavior(SDKAlignedBehavior::ScreenOrientationAPIEnabled); return shouldEnableScreenOrientationAPI; #else return false; #endif }

So yes but it doesn't matter yet to them.

But why take the risk to change the behavior on Watch and TV on the branch at this point?

OK.

I don't mind changing behavior in a follow-up if people think TV and Watch should default to landscape (For TV I could see it make sense, for watch unsure). However, in this patch I'd rather focus on the macOS regression.

Tv’s natural orientation should be landscape, if it even supports this API, but it’s not very important, not enough to change on a branch, certainly (but maybe on trunk).

For iPad, it seems the natural orientation is increasingly evolving to be landscape, but if it already was portrait, it doesn’t seem important to change it late in a release. (Maybe for a later release).

I will follow-up for TV and iPad.

Follow-up: #10803

webkit-early-warning-system · 2023-02-28T02:44:24Z

EWS run on previous version of this PR (hash e75c0a2)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-02-28T16:18:59Z

EWS run on current version of this PR (hash 491d27d)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🛠 gtk~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 api-gtk~~
	~~🛠 tv~~	~~🧪 mac-wk2~~
	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~
	~~🛠 watch~~	~~🧪 mac-wk2-stress~~
	❌ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

https://bugs.webkit.org/show_bug.cgi?id=253026 rdar://105891522 Reviewed by Wenson Hsieh and Darin Adler. The Twitch.tv login was failing because `screen.orientation` was returning "portrait-primary" on macOS. The natural/default orientation on desktop should be "landscape-primary" so this was confusing Twitch. Also update our `screen.angle` logic to take into account the natural orientation based on: - https://w3c.github.io/screen-orientation/#dfn-screen-orientation-values-table I have verified that Chrome on macOS returns "landscape-primary" for the type and 0 for the angle. Our behavior is now aligned. * LayoutTests/fast/screen-orientation/natural-orientation-expected.txt: Added. * LayoutTests/fast/screen-orientation/natural-orientation.html: Added. * LayoutTests/platform/ios/screen-orientation/natural-orientation-expected.txt: Added. * Source/WebCore/page/ScreenOrientation.cpp: (WebCore::ScreenOrientation::lock): * Source/WebCore/page/ScreenOrientationType.h: (WebCore::naturalScreenOrientationType): * Source/WebCore/platform/ScreenOrientationProvider.cpp: (WebCore::ScreenOrientationProvider::currentOrientation): * Source/WebKit/WebProcess/WebCoreSupport/WebScreenOrientationManager.cpp: (WebKit::WebScreenOrientationManager::currentOrientation): Canonical link: https://commits.webkit.org/260944@main

webkit-commit-queue · 2023-02-28T16:26:05Z

Committed 260944@main (541ca5a): https://commits.webkit.org/260944@main

Reviewed commits have been landed. Closing PR #10760 and removing active labels.

cdumez self-assigned this Feb 27, 2023

cdumez added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Feb 27, 2023

cdumez requested review from marcoscaceres and geoffreygaren February 27, 2023 22:26

cdumez requested review from whsieh, achristensen07 and hortont424 February 27, 2023 22:27

whsieh approved these changes Feb 27, 2023

View reviewed changes

darinadler approved these changes Feb 28, 2023

View reviewed changes

marcoscaceres approved these changes Feb 28, 2023

View reviewed changes

cdumez force-pushed the 253026_screen_natural_orientation branch from 2b671ee to e75c0a2 Compare February 28, 2023 02:41

cdumez force-pushed the 253026_screen_natural_orientation branch from e75c0a2 to 491d27d Compare February 28, 2023 16:18

cdumez added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 28, 2023

webkit-commit-queue force-pushed the 253026_screen_natural_orientation branch from 491d27d to 541ca5a Compare February 28, 2023 16:26

webkit-commit-queue merged commit 541ca5a into WebKit:main Feb 28, 2023

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 28, 2023

Trung0246 mentioned this pull request Jun 1, 2023

plugins.twitch: no playable streams on this url 2023-05-31 streamlink/streamlink#5370

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regression(255659@main) Unable to log into twitch.tv #10760

Regression(255659@main) Unable to log into twitch.tv #10760

cdumez commented Feb 27, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Feb 27, 2023 •

edited

darinadler Feb 28, 2023

cdumez Feb 28, 2023

darinadler Feb 28, 2023

darinadler Feb 28, 2023

cdumez Feb 28, 2023 •

edited

darinadler Feb 28, 2023

cdumez Feb 28, 2023

othermaciej Feb 28, 2023

cdumez Feb 28, 2023

cdumez Feb 28, 2023

webkit-early-warning-system commented Feb 28, 2023 •

edited

webkit-early-warning-system commented Feb 28, 2023 •

edited

webkit-commit-queue commented Feb 28, 2023

Regression(255659@main) Unable to log into twitch.tv #10760

Regression(255659@main) Unable to log into twitch.tv #10760

Conversation

cdumez commented Feb 27, 2023 • edited by webkit-early-warning-system

webkit-early-warning-system commented Feb 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdumez Feb 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Feb 28, 2023 • edited

webkit-early-warning-system commented Feb 28, 2023 • edited

webkit-commit-queue commented Feb 28, 2023

cdumez commented Feb 27, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Feb 27, 2023 •

edited

cdumez Feb 28, 2023 •

edited

webkit-early-warning-system commented Feb 28, 2023 •

edited

webkit-early-warning-system commented Feb 28, 2023 •

edited