Closed Bug 1009961 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Panning and Zooming, defect)

28 Branch
ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
tracking-b2g backlog
Tracking Status
b2g-v1.3 --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.1 --- affected
b2g-v2.2 --- fixed

People

(Reporter: ckreinbring, Assigned: kats)

References

()

Details

(Whiteboard: [flame-1.4-exploratory])

Attachments

(3 files, 1 obsolete file)

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
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
Component: Gaia::Browser → Panning and Zooming
Product: Firefox OS → Core
Version: unspecified → 28 Branch
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.
Keywords: qawanted
blocking-b2g: --- → backlog
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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
Keywords: qawanted
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.
Environmental Variables:
Device: Flame 1.4
BuildID: 20140528063006
Gaia: 9d632fd4f8965e15a3204f1609136f1b3b3f8bb3
Gecko: 59f7a4964a77
Version: 30.0
Firmware Version: v10G-2
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.
Depends on: 975084
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.
No longer depends on: 975084
Summary: [B2G] [Browser] Yahoo News article texts do not word wrap and pages do not scroll horizontally → [B2G] [Browser] Some Yahoo News articles fall back to sync scrolling
Attached patch Layers dump (obsolete) — Splinter Review
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.
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.
Attachment #8489437 - Attachment is obsolete: true
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.
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.
Flags: needinfo?(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?
Flags: needinfo?(tnikkel)
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.
Assignee: nobody → bugmail.mozilla
Attached patch PatchSplinter Review
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.
Attachment #8490102 - Flags: review?(tnikkel)
Attachment #8490102 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/3dc40ce2b96c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.