Closed
Bug 726817
Opened 12 years ago
Closed 12 years ago
MAPLE: Websites are cut off
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 13
People
(Reporter: martijn.martijn, Assigned: cwiiis)
References
(Blocks 1 open bug)
Details
(Whiteboard: MAPLE mwc-demo)
Attachments
(4 files, 2 obsolete files)
While testing latest trunk of the Maple branch I noticed that every page is cut off, see screenshot. Latest maple branch can be found here: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-maple/ I also made a video out of this: http://www.youtube.com/watch?v=28GTYb5cFA4&feature=g-upl&context=G2a8c024AUAAAAAAAAAA The screenshots and video are from the LG Optimus Black, but I see this on every device I tested with. I've also seen cases where the page starts scrolling by itself.
Reporter | ||
Comment 1•12 years ago
|
||
After panning, the page gets cut off even more.
Reporter | ||
Comment 2•12 years ago
|
||
I've seen this on several devices, it doesn't happen always, but it happens most of the time. Also weird panning, zooming by itself. It makes the browser very difficult/annoying to work with.
Updated•12 years ago
|
Priority: -- → P1
Reporter | ||
Updated•12 years ago
|
Summary: OMTC: Websites are cut off → MAPLE: Websites are cut off
Affects ICS and the webgl demo on land of the page. Battery and Vibrartion Webapp look missing even though if you tap at certain spots you can bring them up.
![]() |
||
Updated•12 years ago
|
Whiteboard: MAPLE mwc-demo
This also affects Hahlo if you are logged in to it.
Assignee | ||
Comment 6•12 years ago
|
||
Looking at this.
Assignee: nobody → chrislord.net
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•12 years ago
|
||
I see this on pages when you zoom in past 1.0 zoom, for reference.
Assignee | ||
Comment 8•12 years ago
|
||
This is happening because we no longer have any translation to reconcile the scroll position clamp when we zoom. In Java compositor fennec, we would zoom with a CSS zoom, centred on the top-left. The problem with zooming is that it would alter the maximum scrollLeft/scrollTop (the page dimensions haven't changed, so why would it?) - we reconciled this by having an extra translation when we wanted to set scroll coordinates that extended beyond the allowed values. Either we need to employ the same trick again, or we want some kind of flag to set in nsIDOMWindowUtils so we can violate the allowable scroll values. ajuma points out the necessary code to change: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#1811 (nsGfxScrollFrameInner::ClampAndRestrictToDevPixels)
Assignee | ||
Comment 9•12 years ago
|
||
s/would/wouldn't, sorry for the confusion.
Assignee | ||
Comment 10•12 years ago
|
||
Just to note, that although there may well be other problems, I've confirmed that this is *a* problem. Measuring viewport excess, you can see that the scroll position we set is not the one that the window ends up at... I have a patch for turning off scroll position clamping, but it doesn't appear to be working yet.
Assignee | ||
Comment 11•12 years ago
|
||
Fixed this, indeed by adding an option to turn off scroll position clamping. Patches incoming, when Mercurial decides to play ball.
Assignee | ||
Comment 12•12 years ago
|
||
This and the browser.js change on top of it have already been pushed to Maple, but this changes layout so requires review. It adds a new attribute to nsIFrameLoaderOwner called 'clampScrollPosition', in the same vein as clipSubdocument.
Attachment #598410 -
Flags: review?(roc)
Assignee | ||
Comment 13•12 years ago
|
||
And of course immediately I realise that patch isn't quite right - another incoming.
Assignee | ||
Comment 14•12 years ago
|
||
Amended patch.
Attachment #598410 -
Attachment is obsolete: true
Attachment #598410 -
Flags: review?(roc)
Attachment #598433 -
Flags: review?(roc)
Comment on attachment 598433 [details] [diff] [review] Add nsIFrameLoader.clampScrollPosition Review of attachment 598433 [details] [diff] [review]: ----------------------------------------------------------------- r+ with those fixed. ::: content/base/public/nsIFrameLoader.idl @@ +270,5 @@ > + * If false, then the subdocument's scroll coordinates will not be clamped > + * to their scroll boundaries. > + * Defaults to true. > + */ > + attribute boolean clampScrollPosition; rev IID on nsIFrameLoader ::: layout/generic/nsGfxScrollFrame.cpp @@ +1547,5 @@ > nsIScrollableFrame::ScrollMode aMode) > { > + nsSubDocumentFrame* subdocFrame = static_cast<nsSubDocumentFrame*> > + (nsLayoutUtils::GetCrossDocParentFrame(mOuter->PresContext()->PresShell()->GetRootFrame())); > + if (!subdocFrame || subdocFrame->ShouldClampScrollPosition()) Abstract this test into nsGfxScrollFrameInner::ShouldClamp(). Currently, when clampScrollPosition is false, your code makes all scrollable areas in the subdocument not clamp. I'm guessing this is not what you want, especially because your code to start clamping in nsFrameLoader only fixes the scroll position for the root scroll frame. So add a check for mIsRoot here.
Attachment #598433 -
Flags: review?(roc) → review+
Assignee | ||
Comment 16•12 years ago
|
||
Hopefully this addresses all your comments, but I'm not brave enough to assume so, so re-requesting review. Sorry!
Attachment #598433 -
Attachment is obsolete: true
Attachment #598504 -
Flags: review?(roc)
Comment on attachment 598504 [details] [diff] [review] Add nsIFrameLoader.clampScrollPosition (revised) Review of attachment 598504 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsGfxScrollFrame.cpp @@ +1548,5 @@ > { > + if (ShouldClampScrollPosition()) > + mDestination = ClampScrollPosition(aScrollPosition); > + else > + mDestination = aScrollPosition; {} around if/else body
Attachment #598504 -
Flags: review?(roc) → review+
Comment 18•12 years ago
|
||
http://hg.mozilla.org/projects/maple/rev/a4af71f1bc1f
Comment 19•12 years ago
|
||
This seems to have landed.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 22•12 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #19) > This seems to have landed. It has now :-) https://hg.mozilla.org/mozilla-central/rev/e4f5cf4a8f23
Target Milestone: --- → Firefox 13
I think I hvae to wait tomorrow to test on nightly due to bug 729922?
Comment 24•12 years ago
|
||
Verified fixed on: Nightly Fennec 13.0a1 (2012-03-11) Device: HTC Desire Z OS: Android 2.3.3
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•