Taobao.com isn't displayed properly in landscape mode on Android
Categories
(Web Compatibility :: Mobile, defect)
Tracking
(Webcompat Priority:?, firefox68 wontfix, firefox69 wontfix, firefox70 fixed)
Webcompat Priority | ? |
People
(Reporter: ekager, Assigned: emilio)
References
(Regression, )
Details
(Keywords: regression)
Attachments
(5 files)
Copied from: https://github.com/mozilla-mobile/fenix/issues/3898
Can reproduce on Fennec but not Chrome
Steps to reproduce
- Visit taobao.com
- Rotate device to landscape view
Expected behavior
The page is properly displayed, no UI defects.
Actual behavior
The page isn't properly displayed, everything seems to be cornered to the left
Device information
-
Android device:
• Google Pixel 3XL (Android 9)
• Google Pixel 3 (Android 9)
• Pixel Xl (Android Q Beta 4)
• Samsung Galaxy S9 (Android 8)
• Nexus 9 (Android 7.1.1) -
Fenix version: 1.0.1-rc.1 Build # 11851847 4/7
Notes
Comment 1•5 years ago
|
||
I think this is a Gecko layout issue so I'm sending this bug to the Core::Layout component.
This bug was originally reported in the Fenix issue tracker, but Emily says the bug is also reproducible in Fennec.
Assignee | ||
Comment 2•5 years ago
|
||
This is not a layout issue. The width is fixed via CSS.
This is reproducible in RDM in https://h5.m.taobao.com/. If you rotate the device nothing happens, but if you then do window.dispatchEvent(new CustomEvent("resize"))
, then it works. They update the page by literally reloading, which is just amazing.
There are also various suspicious bits about orientation sensors being deprecated.
I've dug a bit more with no clear result, but I think this pretty clearly is not a layout bug. I suspect the url is being reset via location / location.href, but I couldn't verify without a build because those properties are unforgeable.
Mike, is this some issue someone from your team can investigate?
STR are:
- Open https://h5.m.taobao.com on RDM / Fennec / GeckoView.
- Rotate screen, see the page not reload.
If you don't have the cycles and Chris thinks is important, please ni? me and I'll get to it eventually.
Assignee | ||
Comment 3•5 years ago
|
||
Oh, also beware: if you open the page in Chrome (which I did to try to intercept / log the navigation), they try to run xdg-open
, somehow. I haven't tried to open whatever they're trying me to open...
Assignee | ||
Comment 4•5 years ago
|
||
Ah, there you go. In index.min.js:
window.addEventListener("resize",function(){window.innerWidth!==Xe&&window.location.reload()})}]);
Not sure what could be in that Xe
Assignee | ||
Comment 5•5 years ago
|
||
So that's working properly... But why does window.location.reload() doesn't work from the resize listener then?
I recall reading that code... LOL
Here's the issue https://searchfox.org/mozilla-central/rev/b38e3beb658b80e1ed03e0fdf64d225bd4a40327/dom/base/Location.cpp#740
if (window && window->IsHandlingResizeEvent()) {
// location.reload() was called on a window that is handling a
// resize event. Sites do this since Netscape 4.x needed it, but
// we don't, and it's a horrible experience for nothing. In stead
// of reloading the page, just clear style data and reflow the
// page since some sites may use this trick to work around gecko
// reflow bugs, and this should have the same effect.
nsCOMPtr<Document> doc = window->GetExtantDoc();
nsPresContext* pcx;
if (doc && (pcx = doc->GetPresContext())) {
pcx->RebuildAllStyleData(NS_STYLE_HINT_REFLOW,
RestyleHint::RestyleSubtree());
}
return NS_OK;
}
Hah!
Boris, permission to revert bug 258917? I never liked that RebuildAllStyleData call :)
Assignee | ||
Comment 6•5 years ago
|
||
Or alternatively just wontfix this or something, if we believe that RebuildAllStyleData call is worth keeping.
Assignee | ||
Comment 7•5 years ago
|
||
Or putting behind a pref, disable on android and Nightly with the hope of eventually removing or what not.
Updated•5 years ago
|
Comment 8•5 years ago
|
||
This is happening in Firefox 67 to Firefox 70 at least. So indeed unrelated to Bug 1528052, fixed in 68. (as mentioned by hiroyuki on IRC).
Comment 9•5 years ago
|
||
The site once accessed in Firefox Nightly 70 with RDM and Firefox Mobile UA.
Comment 10•5 years ago
|
||
The site once accessed in Firefox Nightly 70 with RDM and Firefox Mobile UA and then rotated in landscape mode.
Comment 11•5 years ago
|
||
And this is what it looks like in Chrome once in landscape mode.
Updated•5 years ago
|
Comment 12•5 years ago
|
||
hiroyuki is also mentioning Bug 1523844 which is in active development.
Assignee | ||
Comment 13•5 years ago
|
||
Those are not related. The site is just handling dynamic resizes by reloading if the width changes. The layout is correct, they just set a bunch of widths to innerWidth.
Comment 14•5 years ago
|
||
Do other browsers generally reload if you reload inside a resize handler?
Because https://html.spec.whatwg.org/multipage/history.html#dom-location-reload says:
If the currently executing task is the dispatch of a resize event in response to the user resizing the browsing context
Repaint the browsing context and return.
Now maybe they are not treating this orientation-change case as "in response to the user resizing the browsing context"? Or maybe they're not following the spec at all here?
Assignee | ||
Comment 15•5 years ago
|
||
Looking at this test-case, it seems they're just not following the spec whatsoever:
<!doctype html>
<script>
onresize = () => location.reload();
onload = () => console.log("load");
</script>
Comment 16•5 years ago
|
||
OK. Well, if we figure the original problem that bug 258917 is no longer an issue, we can go ahead and back it out, I guess, and get the spec changed.
That said, for a desktop user that taobao website will become horrible because opening devtools reloads, resizing the window reloads continuously as you resize, opening the findbar reloads, etc.... Maybe they just do that in their mobile version?
Assignee | ||
Comment 17•5 years ago
|
||
Yes, they only do that on mobile.
Assignee | ||
Comment 18•5 years ago
|
||
I'll write something, yeah... I'm not sure other browsers have TypeAheadFind, so I'll disable only on Desktop Nightly and Mobile for now. If it causes desktop regressions we can just keep the pref on on nightly and off for desktop or something.
Comment 19•5 years ago
|
||
To be clear, the issue with find is not TypeAheadFind per se but that our findbar appearing changes the size of the content area. This is true whether you're using TAF or just Cmd+F/Ctrl+F. See also bug 1281655.
Assignee | ||
Comment 20•5 years ago
|
||
Assignee | ||
Comment 21•5 years ago
|
||
This code was jumping through some hoops that seemed unnecessary. Given
History::Go(0) already calls Location::Reload(false), seems we can just handle
the new pref in one place, and also simplify the code a bit.
Depends on D40430
Comment 22•5 years ago
|
||
Comment 23•5 years ago
|
||
bugherder |
Comment 24•5 years ago
|
||
I assume we want to let this fix ride the trains. Feel free to nominate for uplift if that's not the case, however.
Updated•3 years ago
|
Comment 25•2 years ago
|
||
Emilio, I found another 2+ year old Nightly difference - can this one right the trains out too?
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Description
•