Summary: | [Qt] Editing commands should not be executed on non-editable content. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yael <yael> | ||||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ademar, commit-queue, ossy, staikos, tonikitoo | ||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 46905 | ||||||||||||
Attachments: |
|
Description
Yael
2010-10-08 11:18:53 PDT
Created attachment 70274 [details]
Patch
Remove the calls to editor()->command() when not in editable mode.
SVN blame seems to indicate that this code was originally added in r23545. Would be great to know if this is still needed. Not a Spatial Navigation issue, but it impacts the testing of Spatial Navigation. Example page that shows the problem: <html> <style> </style> <body> <input type="radio" id="myradiobutton"> <br><br><br><br><br><br><br><br><br><br> <span tabindex=1 id="r">My span</span> <br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br> </body> </html> Click the span, and after that, it is not possible to scroll. Callstack was asked for this issue: 0 WebCore::EditorClientQt::handleKeyboardEvent EditorClientQt.cpp 460 0x01e26134 1 WebCore::Editor::handleKeyboardEvent Editor.cpp 125 0x018a454d 2 WebCore::EventHandler::defaultKeyboardEventHandler EventHandler.cpp 2369 0x01b3f838 3 WebCore::Node::defaultEventHandler Node.cpp 2964 0x0182572b 4 WebCore::Node::dispatchGenericEvent Node.cpp 2693 0x018239de 5 WebCore::Node::dispatchEvent Node.cpp 2577 0x018231c8 6 WebCore::EventTarget::dispatchEvent EventTarget.cpp 278 0x01808c63 7 WebCore::EventHandler::keyEvent EventHandler.cpp 2310 0x01b3f06e 8 QWebPagePrivate::keyPressEvent qwebpage.cpp 945 0x01e59396 9 QWebPage::event qwebpage.cpp 2856 0x01e61ca2 10 QWebView::keyPressEvent qwebview.cpp 1033 0x01e673fe 11 QWidget::event qwidget.cpp 8222 0x03723a03 12 QWebView::event qwebview.cpp 834 0x01e66eba 13 QApplicationPrivate::notify_helper qapplication.cpp 4396 0x036b67b6 14 QApplication::notify qapplication.cpp 3857 0x036b42f9 15 QCoreApplication::notifyInternal qcoreapplication.cpp 732 0x0439e62f 16 QCoreApplication::sendSpontaneousEvent qcoreapplication.h 218 0x036b9023 17 qt_sendSpontaneousEvent qapplication.cpp 5388 0x036b6b27 18 QKeyMapper::sendKeyEvent qkeymapper_x11.cpp 1867 0x0378f030 19 QKeyMapperPrivate::translateKeyEvent qkeymapper_x11.cpp 1837 0x0378ed2c 20 QApplication::x11ProcessEvent qapplication_x11.cpp 3429 0x03757d6b ... <More> Created attachment 70289 [details]
Pat
Added test.
In an e-mail exchange with the originator of this code, he was ok with removing it.
Created attachment 70298 [details]
Patch
Modified the test to check that any scrolling happened instead of exactly 40 pixels. Different platforms may have different settings.
(In reply to comment #6) > Created an attachment (id=70289) [details] > Pat > > Added test. > In an e-mail exchange with the originator of this code, he was ok with removing it. The code clean up proposed generally looks good. I am just double checking with Yael if there is not any case not checked by the LayoutTests in Qt's DRT that we might be missing, for example Document::inDesignMode cases. Comment on attachment 70298 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70298&action=review > WebKit/qt/ChangeLog:10 > + A layout test would be very frigile, as the bad behavior is not consistent. Please lets remove this line since we now have a layout test. Created attachment 70530 [details]
Patch
Comment on attachment 70530 [details]
Patch
Removed the line about not having tests, now that we do have a test.
Comment on attachment 70530 [details] Patch Clearing flags on attachment: 70530 Committed r69582: <http://trac.webkit.org/changeset/69582> Revision r69582 cherry-picked into qtwebkit-2.1 with commit a5266b9 <http://gitorious.org/webkit/qtwebkit/commit/a5266b9> |