1009961 - [B2G] [Browser] Some Yahoo News articles fall back to sync scrolling

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Chris Kreinbring [:CKreinbring]]

Attachment: Screenshots of truncated news text

Description:
News articles on the Yahoo browser site that are associated with its News section do not word wrap properly, leaving text unreadable in both portrait and landscape modes.

Repro Steps:
1) Update Flame to Build ID: 20140513000208
2) Launch the Browser and navigate to http://yahoo.com
3) Select a news story associated with Yahoo News (eg Yahoo News, Yahoo Movies, Odd News).
4) Observe the appearance of the article text when the page loads in both portrait and landscape views.

Actual:
The article text runs off the right end of the page, and the user is unable to scroll the page horizontally to view the full text.

Expected:
Either the text is contained within the dimensions of the device screen, or the user is able to scroll the screen horizontally to view the full text.

Environmental Variables
Device: Flame 1.4 mozilla RIL
Build ID: 20140513000208
Gecko: https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/c140bcd02b9b
Gaia: b40103dec34a147c9018a1af76eb21c3184f2f93
Platform Version: 30.0
Firmware Version: v10F-3

Notes:
Repro frequency: 90%
See attached screenshots

Comment 1

[Chris Kreinbring [:CKreinbring]]

Also repros on Buri 1.4 and 1.3 mozilla RILs

Buri 1.4
Build ID: 20140512000204
Gecko: https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ec24f847e7c0
Gaia: 17fb44880e95bc7ae363a609d811bf5a9a067b5b
Platform Version: 30.0
Firmware Version: V1.2-device.cfg

Buri 1.3
Build ID: 20140508024005
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/a7252ab569c4
Gaia: 0d02564946814acfdab13f0b9867d140d318b8ac
Platform Version: 28.0
Firmware Version: V1.2-device.cfg

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Please provide a specific URL on which you can reproduce this. I tried a bunch of yahoo news pages (e.g. https://ca.news.yahoo.com/mass-funerals-mounting-anger-turkey-mourns-mine-workers-191500985.html) but was unable to repro.

Comment 3

[Joshua Mitchell (Inactive)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> Please provide a specific URL on which you can reproduce this. I tried a
> bunch of yahoo news pages (e.g.
> https://ca.news.yahoo.com/mass-funerals-mounting-anger-turkey-mourns-mine-
> workers-191500985.html) but was unable to repro.

The example you provide here is a Routers.com news article linked to from Yahoo.com, I believe the only articles that will repro the issue are Yahoo articles linked to from yahoo.com

Example: https://celebrity.yahoo.com/blogs/celeb-news/fashion-faceoff--kardashian-family-wedding-edition-175210135.html

Comment 4

[Joshua Mitchell (Inactive)]

The issue seems to reproduce only for as long as the page is loading. Once the page is completely loaded the user can scroll to the right to see the rest of the text that is running off the page.

Comment 5

[Joshua Mitchell (Inactive)]

Environmental Variables:
Device: Flame 1.4
BuildID: 20140528063006
Gaia: 9d632fd4f8965e15a3204f1609136f1b3b3f8bb3
Gecko: 59f7a4964a77
Version: 30.0
Firmware Version: v10G-2

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Based on initial investigation I suspect this is just because the content is intercepting touch events and doing the scrolling themselves. However I would need the webpage debugger working to verify.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Actually we appear to be scrolling it based on user input, but for some reason we're falling back to sync scrolling here which we definitely should not be. During page load the main thread is busy and that's why we can't really scroll during that time.

Updating title to reflect new status.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Layers dump

Here's a layers dump of the page in question. There is a container layer with scrollId=8 which has no content under it. There is also no other layer in the tree with scrollId=8, so this is a scrollinfo layer. However that is the layer that scrolls when the user scrolls horizontally, and this results in sync scrolling. I'll get a display list dump as well.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Display list dump + layers dump

Here's one that includes a display list dump. In this one the layer with scrollId=6 is the fishy one. AIUI we shouldn't be running into this case post bug 967844.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Ah, it's because the displayport size is too large so we're skipping it at http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp?rev=861da4eaaa2c#2731 This results in a scrollinfo layer with a non-empty displayport in the FrameMetrics, but the displayport is ignored and so we still fall back to sync scrolling.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

tn, do you think we can take out that code now that we have tiling enabled? The max texture size shouldn't be a limiting factor anymore since we will draw the area as tiles anyway.

Comment 12

[Timothy Nikkel (:tnikkel)]

Do we have tiles everywhere we use displayports and will want to use displayports in the future? Should we make the check conditional on tiling being enabled?

Comment 13

[Kartikaya Gupta (email:[e-mail])]

The code right now is #ifdef'd for b2g anyway, and there we should have tiles everywhere we use displayports. On desktop platforms I don't think 4096 is an appropriate limit anyway since the max texture size might be larger. And on desktop platforms going forward we do plan on implementing tiling before APZ. The only exception would be Metro, where we currently have APZ without tiling, but this code is #ifdef'd out anyway so there would be no change.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This fixes the scrolling issue. However there's still an underlying issue (I think) where the base rect for the displayport is set to 4096 pixels tall when it probably shouldn't be. The base rect getting set so big is what results in the computed displayport from going over 4096 pixels. I'll file a follow-up bug for that.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/b2g-inbound/rev/3dc40ce2b96c

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3dc40ce2b96c