60311 2011-05-05 16:20:16 -0700 [Qt] fast/events/backspace-nagivates-back fails on Qt bots (Mac and Linux) 2011-05-28 05:15:46 -0700 1 1 1 Unclassified WebKit UI Events 528+ (Nightly build) Other Unspecified RESOLVED FIXED P2 Normal --- 1 evan evan commit-queue ojan ossy robert tonikitoo oldest_to_newest 399153 0 evan 2011-05-05 16:20:16 -0700 I just checked in a test that tries to verify that backspace navigates back. The test tests the per-platform behavior for all platforms known by EditingBehavior (mac, win, unix). It fails on Qt. Test output: This test passes if it says PASS below. FAIL: expected navigation to test-complete http://build.webkit.org/results/Qt%20Linux%20Release/r85893%20(32469)/results.html This indicates that somehow pressing "back" *is* causing a navigation. I'm not sure how to diagnose it. Can you help? Should I skip the test to green the tree? 399225 1 tonikitoo 2011-05-05 17:22:22 -0700 > This indicates that somehow pressing "back" *is* causing a navigation. I'm not sure how to diagnose it. > Can you help? Should I skip the test to green the tree? I can have a look. Please skip it, Evan. Ty 399277 2 92525 evan 2011-05-05 18:42:11 -0700 Created attachment 92525 Patch 399279 3 evan 2011-05-05 18:43:42 -0700 I landed this in the skipped list in r85910. http://trac.webkit.org/changeset/85910 399490 4 tonikitoo 2011-05-06 07:15:13 -0700 Evan, it would probably be better to skip it for all qt platforms platform/qt-linux/Skipped is only for Qt on linux. Ossy? 399495 5 ossy 2011-05-06 07:16:37 -0700 (In reply to comment #4) > Evan, it would probably be better to skip it for all qt platforms > > platform/qt-linux/Skipped is only for Qt on linux. Ossy? yes, but we use platform/qt/Skipped because historically reason 399512 6 tonikitoo 2011-05-06 07:47:14 -0700 *** Bug 60349 has been marked as a duplicate of this bug. *** 399528 7 evan 2011-05-06 08:39:16 -0700 Ah, setEditingBehavior isn't implemented! I am glad there is not a more fundamental reason the test doesn't work. 399531 8 tonikitoo 2011-05-06 08:44:56 -0700 (In reply to comment #7) > Ah, setEditingBehavior isn't implemented! I am glad there is not a more fundamental reason the test doesn't work. I implemented that myself :) > grep -nHR setEditingBehavior . ./LayoutTestControllerQt.cpp:771:void LayoutTestController::setEditingBehavior(const QString& editingBehavior) ./LayoutTestControllerQt.cpp:773: DumpRenderTreeSupportQt::setEditingBehavior(m_drt->webPage(), editingBehavior); ./LayoutTestControllerQt.h:251: void setEditingBehavior(const QString& editingBehavior); ./DumpRenderTreeQt.cpp:586: DumpRenderTreeSupportQt::setEditingBehavior(m_page, "win"); 399543 9 ojan 2011-05-06 09:10:50 -0700 (In reply to comment #4) > Evan, it would probably be better to skip it for all qt platforms > > platform/qt-linux/Skipped is only for Qt on linux. Ossy? When we looked at the Qt bots, it was only failing on the Qt-linux bots. Did we misread them? 399551 10 tonikitoo 2011-05-06 09:23:31 -0700 (In reply to comment #8) > (In reply to comment #7) > > Ah, setEditingBehavior isn't implemented! I am glad there is not a more fundamental reason the test doesn't work. > > I implemented that myself :) > > > grep -nHR setEditingBehavior . > ./LayoutTestControllerQt.cpp:771:void LayoutTestController::setEditingBehavior(const QString& editingBehavior) > ./LayoutTestControllerQt.cpp:773: DumpRenderTreeSupportQt::setEditingBehavior(m_drt->webPage(), editingBehavior); > ./LayoutTestControllerQt.h:251: void setEditingBehavior(const QString& editingBehavior); > ./DumpRenderTreeQt.cpp:586: DumpRenderTreeSupportQt::setEditingBehavior(m_page, "win"); Evan, did I miss something? (In reply to comment #9) > (In reply to comment #4) > > Evan, it would probably be better to skip it for all qt platforms > > > > platform/qt-linux/Skipped is only for Qt on linux. Ossy? > > When we looked at the Qt bots, it was only failing on the Qt-linux bots. Did we misread them? The Qt-Mac bot is probably not a core builder nor is in webkit waterfall. see it here: http://build.webkit.sed.hu/waterfall (ossy could you correct me?) 399571 11 evan 2011-05-06 09:46:40 -0700 Oh, my comment was based on this commit message: "* platform/mac-wk2/Skipped: Add fast/events/backspace-navigates-back.html because of missing layoutTestController.setEditingBehavior()." But that must be wk2 specific. Maybe this does fail for some more fundamental issue. :( Does it fail the same way on every bot? 401141 12 ossy 2011-05-10 05:35:00 -0700 (In reply to comment #11) > Oh, my comment was based on this commit message: > "* platform/mac-wk2/Skipped: Add fast/events/backspace-navigates-back.html because of missing layoutTestController.setEditingBehavior()." > > But that must be wk2 specific. WK2 has its own layoutTestController implementation which doesn't have setEditingBehavior() now. > Maybe this does fail for some more fundamental issue. :( > Does it fail the same way on every bot? Yes, Qt Linux, Qt ARM and Qt Mac fails are same. I updated the Skipped lists: http://trac.webkit.org/changeset/86143 404195 13 robert 2011-05-15 11:36:09 -0700 Hi Evan, This is happening because EditorClientQt::handleKeyboardEvent() will only honour the unix editing behaviour (by interpreting the backspace as an edit command) if the selected text is editable. Since this isn't the case here, it ends up in QWebPagePrivate::keyPressEvent(QKeyEvent *ev) which interprets the command as a navigation command. Whereas your test treats it as a no-op, am I correct? I think this just requires a platform-specific result for Qt, since it will always treat a backspace as a page->back() if there is no text to edit - regardless of platform. 404196 14 93588 robert 2011-05-15 11:45:02 -0700 Created attachment 93588 Patch 409851 15 evan 2011-05-25 12:39:08 -0700 Antonio, does this look correct to you? 411833 16 93588 commit-queue 2011-05-28 05:15:40 -0700 Comment on attachment 93588 Patch Clearing flags on attachment: 93588 Committed r87611: <http://trac.webkit.org/changeset/87611> 411834 17 commit-queue 2011-05-28 05:15:46 -0700 All reviewed patches have been landed. Closing bug. 92525 2011-05-05 18:42:11 -0700 2011-05-15 11:44:55 -0700 Patch bug-60311-20110505184335.patch text/plain 1146 evan U3VidmVyc2lvbiBSZXZpc2lvbjogODU5MDkKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCA0ZDQzMjg4MmU4YmNjODc4ZGIxOTM3 YjU1MDYyMGU3NmJkMWZmYmZlLi43MGFkMjVlYzllYTIxMGE4NjRkNWI0YzQ4NTk4NGY4MTMxM2Ew OWMxIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEtMDUtMDUgIEV2YW4gTWFydGluICA8ZXZh bkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgTmV3IHRlc3QgZmFzdC9ldmVudHMvYmFja3NwYWNlLW5hZ2l2YXRlcy1iYWNrIGZh aWxzIG9uIFF0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD02MDMxMQorCisgICAgICAgIFNraXAgdGhpcyB0ZXN0IHVudGlsIEFudG9ub2kgaGFzIHRpbWUg dG8gbG9vayBhdCBpdC4KKworICAgICAgICAqIHBsYXRmb3JtL3F0LWxpbnV4L1NraXBwZWQ6CisK IDIwMTEtMDUtMDUgIEplciBOb2JsZSAgPGplci5ub2JsZUBhcHBsZS5jb20+CiAKICAgICAgICAg VW5yZXZpZXdlZCwgcGFydGlhbCByb2xsb3V0IG9mIHI4NTg0MyBhbmQgcjg1ODI0LiAgcjg1ODk3 IG1hZGUgdGhlc2UgZXhwZWN0ZWQgcmVzdWx0cyB1bm5lY2Vzc2FyeS4KZGlmZiAtLWdpdCBhL0xh eW91dFRlc3RzL3BsYXRmb3JtL3F0LWxpbnV4L1NraXBwZWQgYi9MYXlvdXRUZXN0cy9wbGF0Zm9y bS9xdC1saW51eC9Ta2lwcGVkCmluZGV4IGU2OWRlMjliYjJkMWQ2NDM0YjhiMjlhZTc3NWFkOGMy ZTQ4YzUzOTEuLmZkMWQ4YzFlODllN2JmMDg2ZGI1YzlkNDdhM2ZlYmRmOWY0NjA3M2EgMTAwNjQ0 Ci0tLSBhL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0LWxpbnV4L1NraXBwZWQKKysrIGIvTGF5b3V0 VGVzdHMvcGxhdGZvcm0vcXQtbGludXgvU2tpcHBlZApAQCAtMCwwICsxLDMgQEAKKyMgTmV3IHRl c3QgZmFpbHMgb24gUXQsIExpbnV4IG9ubHk/CisjIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD02MDMxMQorZmFzdC9ldmVudHMvYmFja3NwYWNlLW5hdmlnYXRlcy1iYWNr Lmh0bWwK 93588 2011-05-15 11:45:02 -0700 2011-05-28 05:15:40 -0700 Patch bug-60311-20110515194501.patch text/plain 1256 robert U3VidmVyc2lvbiBSZXZpc2lvbjogODY1MDUKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCA5ODAxZDY4NGMxZjg2ZjU1NmVhNjc5 YTBlOTBlNjBhMmU1Y2IxM2NkLi5iZTU1YTNkODAzYjI4NDZhMDExMDU5OWEwYzc3MWQyY2IwNTEx M2VjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDUtMTUgIFJvYmVydCBIb2dhbiAgPHJv YmVydEB3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFtRdF0gZmFzdC9ldmVudHMvYmFja3NwYWNlLW5hZ2l2YXRlcy1iYWNrIGZhaWxz IG9uIFF0IGJvdHMgKE1hYyBhbmQgTGludXgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD02MDMxMQorCisgICAgICAgIElmIHRoZXJlIGlzIG5vIGVkaXRh YmxlIHRleHQgUXQgaW50ZXJwcmV0cyBiYWNrc3BhY2UgYXMgYSBuYXZpZ2F0aW9uCisgICAgICAg IGNvbW1hbmQsIHJlZ2FyZGxlc3Mgb2YgcGxhdGZvcm0uCisKKyAgICAgICAgKiBwbGF0Zm9ybS9x dC9mYXN0L2V2ZW50cy9iYWNrc3BhY2UtbmF2aWdhdGVzLWJhY2stZXhwZWN0ZWQudHh0OiBBZGRl ZC4KKwogMjAxMS0wNS0xNSAgU2hlcmlmZiBCb3QgIDx3ZWJraXQucmV2aWV3LmJvdEBnbWFpbC5j b20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjg2NTA0LgpkaWZmIC0tZ2l0 IGEvTGF5b3V0VGVzdHMvcGxhdGZvcm0vcXQvZmFzdC9ldmVudHMvYmFja3NwYWNlLW5hdmlnYXRl cy1iYWNrLWV4cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L2Zhc3QvZXZlbnRz L2JhY2tzcGFjZS1uYXZpZ2F0ZXMtYmFjay1leHBlY3RlZC50eHQKbmV3IGZpbGUgbW9kZSAxMDA2 NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uNTc0NzMx NGVlNGRjODYxOThmNGZkNGFmZTI2MzI5MjFlMWYyNjgxYQotLS0gL2Rldi9udWxsCisrKyBiL0xh eW91dFRlc3RzL3BsYXRmb3JtL3F0L2Zhc3QvZXZlbnRzL2JhY2tzcGFjZS1uYXZpZ2F0ZXMtYmFj ay1leHBlY3RlZC50eHQKQEAgLTAsMCArMSwyIEBACitUaGlzIHRlc3QgcGFzc2VzIGlmIGl0IHNh eXMgUEFTUyBiZWxvdy4KK0ZBSUw6IGV4cGVjdGVkIG5hdmlnYXRpb24gdG8gdGVzdC1jb21wbGV0 ZQo=