Closed Bug 1570566 Opened 1 year ago Closed 1 year ago

Taobao.com isn't displayed properly in landscape mode on Android

Categories

(Web Compatibility :: Mobile, defect)

Unspecified
Android
defect
Not set

Tracking

(Webcompat Priority:?, firefox68 wontfix, firefox69 wontfix, firefox70 fixed)

RESOLVED FIXED
Webcompat Priority ?
Tracking Status
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: ekager, Assigned: emilio)

References

(Regression, )

Details

Attachments

(5 files)

Copied from: https://github.com/mozilla-mobile/fenix/issues/3898
Can reproduce on Fennec but not Chrome

Steps to reproduce

  1. Visit taobao.com
  2. 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

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.

https://github.com/mozilla-mobile/fenix/issues/3898

Component: General → Layout
OS: All → Android
Product: GeckoView → Core
Summary: Taobao.com isn't displayed properly in landscape mode → Taobao.com isn't displayed properly in landscape mode on Android

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:

If you don't have the cycles and Chris thinks is important, please ni? me and I'll get to it eventually.

Component: Layout → Mobile
Flags: needinfo?(miket)
Product: Core → Web Compatibility
Version: 69 Branch → unspecified

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...

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

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 :)

Flags: needinfo?(miket) → needinfo?(bzbarsky)
Regressed by: 258917

Or alternatively just wontfix this or something, if we believe that RebuildAllStyleData call is worth keeping.

Or putting behind a pref, disable on android and Nightly with the hope of eventually removing or what not.

Webcompat Priority: --- → ?

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).

Attached image before-rotation

The site once accessed in Firefox Nightly 70 with RDM and Firefox Mobile UA.

Attached image after-rotation

The site once accessed in Firefox Nightly 70 with RDM and Firefox Mobile UA and then rotated in landscape mode.

And this is what it looks like in Chrome once in landscape mode.

hiroyuki is also mentioning Bug 1523844 which is in active development.

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.

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?

Flags: needinfo?(bzbarsky)

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>

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?

Yes, they only do that on mobile.

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.

Assignee: nobody → emilio

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.

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

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17ea07407731
Don't block reloading during a resize event handler on Android and Nightly. r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/c4265d8d0301
Cleanup a bit Location::Reload() and History::Go(0). r=bzbarsky
No longer depends on: 1571756

I assume we want to let this fix ride the trains. Feel free to nominate for uplift if that's not the case, however.

You need to log in before you can comment on or make changes to this bug.