51318 2010-12-19 21:14:37 -0800 Make FrameView::calculateScrollbarModesForLayout private again. 2010-12-21 07:12:18 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED INVALID P2 Normal --- 1 tonikitoo tonikitoo hyatt simon.fraser yael oldest_to_newest 324284 0 tonikitoo 2010-12-19 21:14:37 -0800 Method was made public by r72522, however after stuff its code and its code path, I do not think we need have it as public. Instead, we can rely on the already existent and public ScrollView::scrollbarModes method. Reason: in spatial navigation, before any move we have the following call stack: Document::updateLayoutIgnorePendingStylesheets -> Document::updateLayout -> FrameView::layout -> FrameView::calculateScrollbarModesForLayout -> ScrollView::setScrolbarModes. That means we have already the proper scrollbar modes set, and we can just query for it with ScrollView::scrollbarModes when needed. All tests pass. 324285 1 76974 tonikitoo 2010-12-19 21:20:08 -0800 Created attachment 76974 patch v1 324353 2 yael 2010-12-20 06:24:32 -0800 Antonio, Did you test this with an application that explicitly sets the scrollbar mode to off via the WebKit API? 324354 3 tonikitoo 2010-12-20 06:26:15 -0800 (In reply to comment #2) > Antonio, Did you test this with an application that explicitly sets the scrollbar mode to off via the WebKit API? I will make this test before landing. If it turns out it is needed, I have another patch with such a test coming... 324778 4 tonikitoo 2010-12-20 21:41:15 -0800 Yael, what is the desired behavior here: Should spatial navigation perform scrolling even we set ScrollBarPolicy::AlwaysOff? Note: when we set scrollbarpolicy::alwaysoff, qwebpage does not scroll with arrow keys (see handle scroll method). Should we be consistent? I have a layouttest can sets scrollbar policy (Qt specific, since Qt is the only DRT that supports it). I am wondering if we should scroll or not. 324969 5 yael 2010-12-21 05:52:39 -0800 (In reply to comment #4) > Yael, what is the desired behavior here: Should spatial navigation perform scrolling even we set ScrollBarPolicy::AlwaysOff? > > Note: when we set scrollbarpolicy::alwaysoff, qwebpage does not scroll with arrow keys (see handle scroll method). Should we be consistent? > > I have a layouttest can sets scrollbar policy (Qt specific, since Qt is the only DRT that supports it). I am wondering if we should scroll or not. I believe we should scroll when scrollbars are set to off. The reason is that both spatial navigation and turning off the scrollbars seem to be more mobile oriented features than desktop features. A modern mobile browser would not want scrollbars on the main frame, but would want to scroll the content of that main frame. 324989 6 tonikitoo 2010-12-21 07:06:47 -0800 > I believe we should scroll when scrollbars are set to off. > The reason is that both spatial navigation and turning off the scrollbars seem to be more mobile oriented features than desktop features. A modern mobile browser would not want scrollbars on the main frame, but would want to scroll the content of that main frame. Ok, lets change this bug then. Patch with a layout test coming ... 324994 7 tonikitoo 2010-12-21 07:12:18 -0800 (In reply to comment #5) > (In reply to comment #4) > > Yael, what is the desired behavior here: Should spatial navigation perform scrolling even we set ScrollBarPolicy::AlwaysOff? > > > > Note: when we set scrollbarpolicy::alwaysoff, qwebpage does not scroll with arrow keys (see handle scroll method). Should we be consistent? > > > > I have a layouttest can sets scrollbar policy (Qt specific, since Qt is the only DRT that supports it). I am wondering if we should scroll or not. > > I believe we should scroll when scrollbars are set to off. > The reason is that both spatial navigation and turning off the scrollbars seem to be more mobile oriented features than desktop features. A modern mobile browser would not want scrollbars on the main frame, but would want to scroll the content of that main frame. bug 51396. 76974 2010-12-19 21:20:08 -0800 2010-12-21 07:08:58 -0800 patch v1 0001-2010-12-19-Antonio-Gomes-agomes-rim.com.patch text/plain 4556 tonikitoo RnJvbSA0ZDVkNTZhOWM2MjA0YWRhY2VmYWE1NDZlZjhjNDVjNjc3ZmIwOTBiIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbnRvbmlvIEdvbWVzIDxhZ29tZXNAcmltLmNvbT4KRGF0ZTog TW9uLCAyMCBEZWMgMjAxMCAwMDoxOTowMyAtMDUwMApTdWJqZWN0OiBbUEFUQ0hdIDIwMTAtMTIt MTkgIEFudG9uaW8gR29tZXMgIDxhZ29tZXNAcmltLmNvbT4KCiAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCgogICAgICAgIE1ha2UgRnJhbWVWaWV3OjpjYWxjdWxhdGVTY3JvbGxi YXJNb2Rlc0ZvckxheW91dCBwcml2YXRlIGFnYWluLgogICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD01MTMxOAoKICAgICAgICBNZXRob2Qgd2FzIG1hZGUgcHVi bGljIGJ5IHI3MjUyMiwgaG93ZXZlciBhZnRlciBzdHVkeSBpdHMgY29kZSBhbmQgaXRzIGNvZGUK ICAgICAgICBwYXRoLCBpdCB0dXJucyBvdXQgd2UgZG8gbm8gbmVlZCBpdCBhcyBwdWJsaWMuIElu c3RlYWQsIHdlIGNhbiByZWx5IG9uIHRoZQogICAgICAgIGFscmVhZHkgZXhpc3RlbnQgYW5kIHB1 YmxpYyBTY3JvbGxWaWV3OjpzY3JvbGxiYXJNb2RlcyBtZXRob2QuCgogICAgICAgIFJlYXNvbjog aW4gc3BhdGlhbCBuYXZpZ2F0aW9uLCBiZWZvcmUgYW55IG1vdmUgd2UgaGF2ZSB0aGUgZm9sbG93 aW5nIGNhbGwKICAgICAgICBzdGFjazoKICAgICAgICAtIERvY3VtZW50Ojp1cGRhdGVMYXlvdXRJ Z25vcmVQZW5kaW5nU3R5bGVzaGVldHMgY2FsbHMKICAgICAgICAtIERvY3VtZW50Ojp1cGRhdGVM YXlvdXQgY2FsbHMKICAgICAgICAtIEZyYW1lVmlldzo6bGF5b3V0IGNhbGxzCiAgICAgICAgLSBG cmFtZVZpZXc6OmNhbGN1bGF0ZVNjcm9sbGJhck1vZGVzRm9yTGF5b3V0IGNhbGxzCiAgICAgICAg LSBTY3JvbGxWaWV3OjpzZXRTY3JvbGJhck1vZGVzLgoKICAgICAgICBUaGF0IG1lYW5zIHdlIGhh dmUgYWxyZWFkeSB0aGUgcHJvcGVyIHNjcm9sbGJhciBtb2RlcyBzZXQsIGFuZCB3ZSBjYW4ganVz dAogICAgICAgIHF1ZXJ5IGZvciBpdCB3aXRoIFNjcm9sbFZpZXc6OnNjcm9sbGJhck1vZGVzIHdo ZW4gbmVlZGVkLgoKICAgICAgICBObyBuZXcgdGVzdHMgbmVlZGVkLgoKICAgICAgICAqIHBhZ2Uv RnJhbWVWaWV3Lmg6CiAgICAgICAgKiBwYWdlL1NwYXRpYWxOYXZpZ2F0aW9uLmNwcDoKICAgICAg ICAoV2ViQ29yZTo6Y2FuU2Nyb2xsSW5EaXJlY3Rpb24pOgotLS0KIFdlYkNvcmUvQ2hhbmdlTG9n ICAgICAgICAgICAgICAgICAgfCAgIDI5ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrCiBX ZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmggICAgICAgICAgIHwgICAgMyArLS0KIFdlYkNvcmUvcGFn ZS9TcGF0aWFsTmF2aWdhdGlvbi5jcHAgfCAgICAzICsrLQogMyBmaWxlcyBjaGFuZ2VkLCAzMiBp bnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdl TG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggM2JlYWY5Ni4uMzhkNjcyOCAxMDA2NDQKLS0t IGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwz MiBAQAorMjAxMC0xMi0xOSAgQW50b25pbyBHb21lcyAgPGFnb21lc0ByaW0uY29tPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE1ha2UgRnJhbWVWaWV3 OjpjYWxjdWxhdGVTY3JvbGxiYXJNb2Rlc0ZvckxheW91dCBwcml2YXRlIGFnYWluLgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTEzMTgKKworICAgICAg ICBNZXRob2Qgd2FzIG1hZGUgcHVibGljIGJ5IHI3MjUyMiwgaG93ZXZlciBhZnRlciBzdHVkeSBp dHMgY29kZSBhbmQgaXRzIGNvZGUKKyAgICAgICAgcGF0aCwgaXQgdHVybnMgb3V0IHdlIGRvIG5v IG5lZWQgaXQgYXMgcHVibGljLiBJbnN0ZWFkLCB3ZSBjYW4gcmVseSBvbiB0aGUKKyAgICAgICAg YWxyZWFkeSBleGlzdGVudCBhbmQgcHVibGljIFNjcm9sbFZpZXc6OnNjcm9sbGJhck1vZGVzIG1l dGhvZC4KKworICAgICAgICBSZWFzb246IGluIHNwYXRpYWwgbmF2aWdhdGlvbiwgYmVmb3JlIGFu eSBtb3ZlIHdlIGhhdmUgdGhlIGZvbGxvd2luZyBjYWxsCisgICAgICAgIHN0YWNrOgorCisgICAg ICAgIC0gRG9jdW1lbnQ6OnVwZGF0ZUxheW91dElnbm9yZVBlbmRpbmdTdHlsZXNoZWV0cyBjYWxs cworICAgICAgICAtIERvY3VtZW50Ojp1cGRhdGVMYXlvdXQgY2FsbHMKKyAgICAgICAgLSBGcmFt ZVZpZXc6OmxheW91dCBjYWxscworICAgICAgICAtIEZyYW1lVmlldzo6Y2FsY3VsYXRlU2Nyb2xs YmFyTW9kZXNGb3JMYXlvdXQgY2FsbHMKKyAgICAgICAgLSBTY3JvbGxWaWV3OjpzZXRTY3JvbGJh ck1vZGVzLgorCisgICAgICAgIFRoYXQgbWVhbnMgd2UgaGF2ZSBhbHJlYWR5IHRoZSBwcm9wZXIg c2Nyb2xsYmFyIG1vZGVzIHNldCwgYW5kIHdlIGNhbiBqdXN0CisgICAgICAgIHF1ZXJ5IGZvciBp dCB3aXRoIFNjcm9sbFZpZXc6OnNjcm9sbGJhck1vZGVzIHdoZW4gbmVlZGVkLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cyBuZWVkZWQuCisKKyAgICAgICAgKiBwYWdlL0ZyYW1lVmlldy5oOgorICAg ICAgICAqIHBhZ2UvU3BhdGlhbE5hdmlnYXRpb24uY3BwOgorICAgICAgICAoV2ViQ29yZTo6Y2Fu U2Nyb2xsSW5EaXJlY3Rpb24pOgorCiAyMDEwLTEyLTE5ICBEYW4gQmVybnN0ZWluICA8bWl0ekBh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQ2FtZXJvbiBad2FyaWNoLgpkaWZmIC0t Z2l0IGEvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5oIGIvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5o CmluZGV4IGQxMzI5MTYuLmU2OWZhM2EgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGFnZS9GcmFtZVZp ZXcuaAorKysgYi9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmgKQEAgLTIzNSw4ICsyMzUsNiBAQCBw dWJsaWM6CiAgICAgYm9vbCBpc0ZyYW1lVmlld1Njcm9sbENvcm5lcihSZW5kZXJTY3JvbGxiYXJQ YXJ0KiBzY3JvbGxDb3JuZXIpIGNvbnN0IHsgcmV0dXJuIG1fc2Nyb2xsQ29ybmVyID09IHNjcm9s bENvcm5lcjsgfQogICAgIHZvaWQgaW52YWxpZGF0ZVNjcm9sbENvcm5lcigpOwogCi0gICAgdm9p ZCBjYWxjdWxhdGVTY3JvbGxiYXJNb2Rlc0ZvckxheW91dChTY3JvbGxiYXJNb2RlJiBoTW9kZSwg U2Nyb2xsYmFyTW9kZSYgdk1vZGUpOwotCiAgICAgLy8gTm9ybWFsIGRlbGF5CiAgICAgc3RhdGlj IHZvaWQgc2V0UmVwYWludFRocm90dGxpbmdEZWZlcnJlZFJlcGFpbnREZWxheShkb3VibGUgcCk7 CiAgICAgLy8gTmVnYXRpdmUgdmFsdWUgd291bGQgbWVhbiB0aGF0IGZpcnN0IGZldyByZXBhaW50 cyBoYXBwZW4gd2l0aG91dCBhIGRlbGF5CkBAIC0yNjksNiArMjY3LDcgQEAgcHJpdmF0ZToKICAg ICBib29sIGhhc0ZpeGVkT2JqZWN0cygpIGNvbnN0IHsgcmV0dXJuIG1fZml4ZWRPYmplY3RDb3Vu dCA+IDA7IH0KIAogICAgIHZvaWQgYXBwbHlPdmVyZmxvd1RvVmlld3BvcnQoUmVuZGVyT2JqZWN0 KiwgU2Nyb2xsYmFyTW9kZSYgaE1vZGUsIFNjcm9sbGJhck1vZGUmIHZNb2RlKTsKKyAgICB2b2lk IGNhbGN1bGF0ZVNjcm9sbGJhck1vZGVzRm9yTGF5b3V0KFNjcm9sbGJhck1vZGUmIGhNb2RlLCBT Y3JvbGxiYXJNb2RlJiB2TW9kZSk7CiAKICAgICB2b2lkIHVwZGF0ZU92ZXJmbG93U3RhdHVzKGJv b2wgaG9yaXpvbnRhbE92ZXJmbG93LCBib29sIHZlcnRpY2FsT3ZlcmZsb3cpOwogCmRpZmYgLS1n aXQgYS9XZWJDb3JlL3BhZ2UvU3BhdGlhbE5hdmlnYXRpb24uY3BwIGIvV2ViQ29yZS9wYWdlL1Nw YXRpYWxOYXZpZ2F0aW9uLmNwcAppbmRleCA1Y2IzYzk1Li40NzJiNWY5IDEwMDY0NAotLS0gYS9X ZWJDb3JlL3BhZ2UvU3BhdGlhbE5hdmlnYXRpb24uY3BwCisrKyBiL1dlYkNvcmUvcGFnZS9TcGF0 aWFsTmF2aWdhdGlvbi5jcHAKQEAgLTQ3NCw3ICs0NzQsOCBAQCBib29sIGNhblNjcm9sbEluRGly ZWN0aW9uKEZvY3VzRGlyZWN0aW9uIGRpcmVjdGlvbiwgY29uc3QgRnJhbWUqIGZyYW1lKQogICAg ICAgICByZXR1cm4gZmFsc2U7CiAgICAgU2Nyb2xsYmFyTW9kZSB2ZXJ0aWNhbE1vZGU7CiAgICAg U2Nyb2xsYmFyTW9kZSBob3Jpem9udGFsTW9kZTsKLSAgICBmcmFtZS0+dmlldygpLT5jYWxjdWxh dGVTY3JvbGxiYXJNb2Rlc0ZvckxheW91dChob3Jpem9udGFsTW9kZSwgdmVydGljYWxNb2RlKTsK KyAgICBmcmFtZS0+dmlldygpLT5zY3JvbGxiYXJNb2Rlcyhob3Jpem9udGFsTW9kZSwgdmVydGlj YWxNb2RlKTsKKwogICAgIGlmICgoZGlyZWN0aW9uID09IEZvY3VzRGlyZWN0aW9uTGVmdCB8fCBk aXJlY3Rpb24gPT0gRm9jdXNEaXJlY3Rpb25SaWdodCkgJiYgU2Nyb2xsYmFyQWx3YXlzT2ZmID09 IGhvcml6b250YWxNb2RlKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgaWYgKChkaXJlY3Rp b24gPT0gRm9jdXNEaXJlY3Rpb25VcCB8fCBkaXJlY3Rpb24gPT0gRm9jdXNEaXJlY3Rpb25Eb3du KSAmJiAgU2Nyb2xsYmFyQWx3YXlzT2ZmID09IHZlcnRpY2FsTW9kZSkKLS0gCjEuNy4xCgo=