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?
> 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
Created attachment 92525 [details] Patch
I landed this in the skipped list in r85910. http://trac.webkit.org/changeset/85910
Evan, it would probably be better to skip it for all qt platforms platform/qt-linux/Skipped is only for Qt on linux. Ossy?
(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
*** Bug 60349 has been marked as a duplicate of this bug. ***
Ah, setEditingBehavior isn't implemented! I am glad there is not a more fundamental reason the test doesn't work.
(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");
(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?
(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?)
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?
(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
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.
Created attachment 93588 [details] Patch
Antonio, does this look correct to you?
Comment on attachment 93588 [details] Patch Clearing flags on attachment: 93588 Committed r87611: <http://trac.webkit.org/changeset/87611>
All reviewed patches have been landed. Closing bug.