Bug 115178 - [BlackBerry] Make scroll position adjustment work with pages with fixed position elements
Summary: [BlackBerry] Make scroll position adjustment work with pages with fixed posit...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-25 08:11 PDT by Iris Wu
Modified: 2013-05-02 08:51 PDT (History)
3 users (show)

See Also:


Attachments
Patch with ChangeLog (6.25 KB, patch)
2013-04-25 08:36 PDT, Iris Wu
no flags Details | Formatted Diff | Diff
Patch with ChangeLog (7.25 KB, patch)
2013-04-25 11:08 PDT, Iris Wu
no flags Details | Formatted Diff | Diff
Patch with ChangeLog (7.23 KB, patch)
2013-04-25 11:13 PDT, Iris Wu
no flags Details | Formatted Diff | Diff
debug build fix (1.95 KB, patch)
2013-04-26 07:46 PDT, Iris Wu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Iris Wu 2013-04-25 08:11:16 PDT
Currently the position smart scroll adjusts is the top left point of the viewport. On the page with fixed position elements, we want it to adjust the position beneath the fixed elements so it can be always visible.
Comment 1 Iris Wu 2013-04-25 08:36:59 PDT
Created attachment 199664 [details]
Patch with ChangeLog
Comment 2 Rob Buis 2013-04-25 09:01:29 PDT
Comment on attachment 199664 [details]
Patch with ChangeLog

View in context: https://bugs.webkit.org/attachment.cgi?id=199664&action=review

> Source/WebKit/blackberry/Api/WebPage.cpp:5255
> +void WebPage::findFixedElementRect(LayerCompositingThread* layer, Platform::IntRect& fixedElementRect)

It seems like this method should be on LayerCompositingThread, since every operation uses layer.
Comment 3 Arvid Nilsson 2013-04-25 09:27:16 PDT
(In reply to comment #2)
> (From update of attachment 199664 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199664&action=review
> 
> > Source/WebKit/blackberry/Api/WebPage.cpp:5255
> > +void WebPage::findFixedElementRect(LayerCompositingThread* layer, Platform::IntRect& fixedElementRect)
> 
> It seems like this method should be on LayerCompositingThread, since every operation uses layer.

I was thinking of suggesting to put this method on WebPageCompositorPrivate, at least. Maybe moving it to LayerCompositingThread makes even more sense - but I think traversing the layers should be the responsibility of some managing class like LayerRenderer or WebPageCompositor(Private), not LayerCompositingThread itself.
Comment 4 Iris Wu 2013-04-25 11:08:00 PDT
Created attachment 199689 [details]
Patch with ChangeLog
Comment 5 WebKit Commit Bot 2013-04-25 11:09:51 PDT
Attachment 199689 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/WebPage.cpp', u'Source/WebKit/blackberry/Api/WebPage.h', u'Source/WebKit/blackberry/Api/WebPageCompositor.cpp', u'Source/WebKit/blackberry/Api/WebPageCompositor_p.h', u'Source/WebKit/blackberry/ChangeLog']" exit_code: 1
Source/WebKit/blackberry/Api/WebPageCompositor_p.h:107:  The parameter name "layer" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Iris Wu 2013-04-25 11:13:00 PDT
Created attachment 199693 [details]
Patch with ChangeLog
Comment 7 Rob Buis 2013-04-25 11:22:49 PDT
Comment on attachment 199693 [details]
Patch with ChangeLog

LGTM.
Comment 8 WebKit Commit Bot 2013-04-25 13:01:50 PDT
Comment on attachment 199693 [details]
Patch with ChangeLog

Clearing flags on attachment: 199693

Committed r149137: <http://trac.webkit.org/changeset/149137>
Comment 9 WebKit Commit Bot 2013-04-25 13:01:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Iris Wu 2013-04-26 07:46:12 PDT
Created attachment 199835 [details]
debug build fix
Comment 11 Iris Wu 2013-04-30 15:27:16 PDT
New patch added
Comment 12 Rob Buis 2013-05-02 08:09:48 PDT
Comment on attachment 199835 [details]
debug build fix

Ok.
Comment 13 WebKit Commit Bot 2013-05-02 08:51:43 PDT
Comment on attachment 199835 [details]
debug build fix

Clearing flags on attachment: 199835

Committed r149487: <http://trac.webkit.org/changeset/149487>
Comment 14 WebKit Commit Bot 2013-05-02 08:51:45 PDT
All reviewed patches have been landed.  Closing bug.