Bug 166241

Summary: [regression][testcase] Double-click word selection in Konqueror often selects multiple words or only a part of a word
Product: [Applications] konqueror Reporter: Saurav Sarkar <saurav.sarkar>
Component: khtmlAssignee: Konqueror Developers <konq-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: amantia, aros, divided.mind, esigra, finex, frank78ac, germain, jem, kollix, mtadeunet, tsjoker
Priority: NOR    
Version: SVN   
Target Milestone: ---   
Platform: Ubuntu   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: Double-click word selection in Konqueror
Double-click word selection in Konqueror
Double-click word selection in Konqueror
Simple testcase.
Improved test case
convert Selection's DOM offsets to Rendered offsets, as this is what we need here.

Description Saurav Sarkar 2008-07-10 17:36:16 UTC
Version:            (using KDE 4.0.83)
Installed from:    Ubuntu Packages

If we double-click on a word in a web page in konqueror, the word should be selected. This is often the case but not always. For example opening http://www.artima.com/cppsource/top_cpp_books.html in konqueror and double-clicking many words (e.g. audacious) selects either only a part of the word or even some parts of two adjacent words. The selection varies depending on which part of the word was double-clicked. See attached screenshots.
Comment 1 Saurav Sarkar 2008-07-10 17:37:37 UTC
Created attachment 26014 [details]
Double-click word selection in Konqueror
Comment 2 Saurav Sarkar 2008-07-10 17:38:28 UTC
Created attachment 26015 [details]
Double-click word selection in Konqueror
Comment 3 Saurav Sarkar 2008-07-10 17:39:14 UTC
Created attachment 26016 [details]
Double-click word selection in Konqueror
Comment 4 Frank Reininghaus 2008-07-10 20:32:29 UTC
Thanks for the bug report. I can confirm this with SVN trunk rev. 830161. Konqueror 4.0.5 on Kubuntu 8.04 seems to work fine.
Comment 5 Frank Reininghaus 2008-07-10 20:34:18 UTC
Created attachment 26020 [details]
Simple testcase.
Comment 6 Frank Reininghaus 2008-07-10 20:58:19 UTC
The funny thing is that double-click selection works fine for me in this bug report page, but not in the test case. Needs further analysis...
Comment 7 Frank Reininghaus 2008-07-12 00:40:59 UTC
Created attachment 26050 [details]
Improved test case

OK, I think I know what's going on. Extra spaces in front of words make text
selection go wrong, i.e., spaces at the beginning of a line or multiple spaces
between words. These spaces are not shown, but the text selection algorithm
does not take this into account.

Konqueror 4.0.5 still works OK for me.
Comment 8 Frank Reininghaus 2008-07-12 02:52:30 UTC
I browsed through the code and got the impression that this regression was probably caused by commit 809453:
http://websvn.kde.org/?view=rev&revision=809453
I'm CC'ing the developer who committed it.

I verified that it still works in 809421 and that the bug is there in 809459. I was unable to build the revisions in between.

By the way, I had to do my testing with an offline copy of the testcase (after doing "svn up --revision ..." and building khtml and konqueror) because web browsing didn't work: I got a page saying "The requested operation could not be completed - Unexpected Program Termination..." in Konqueror. If anyone knows what I did wrong, please let me know.

Comment 9 Frank Reininghaus 2008-07-17 01:45:03 UTC
I've looked a bit further into this. I have to stop now, so I'm posting my intermediate results here, just in case anyone else has good ideas:

The selection seems to be done in Selection::validate() [granularity == WORD] in kdelibs/khtml/xml/dom_selection.cpp. This function calls findWordBoundary() from kdelibs/khtml/misc/helper.cpp. The parameter 'position' of this function should probably be the index in the array 'chars' which corresponds to the clicked character. This is not the case if there are extra spaces in front of the corresponding word. This means that the calculation of this position goes wrong.

This calculation takes place in KHTMLPart::handleMousePressEventDoubleClick() in kdelibs/khtml/khtml_part.cpp in the line 

Position pos(innerNode.handle()->positionForCoordinates(event->x(), event->y()));

Comment 10 Frank Reininghaus 2008-10-24 19:53:10 UTC
*** Bug 173413 has been marked as a duplicate of this bug. ***
Comment 11 FiNeX 2008-11-27 11:47:44 UTC
I've just reproduced the selection problem using current trunk ( ~ 4.2beta1)
Comment 12 duanedesign 2009-01-07 05:27:14 UTC
This bug has been reported on Launchpad:
https://bugs.edge.launchpad.net/kdebase/+bug/301707

Binary package hint: konqueror

Hi when I double-click on a word in Konqueror I have a strange behavior. Please check the attachments( png's ) and their comments

(screenshot)  double-click on 'b' of the 'bug' word  (16.2 KiB, image/png)
http://launchpadlibrarian.net/19906687/screen1.png

(screenshot) double-click on 'o' of 'one'  (15.3 KiB, image/png)
http://launchpadlibrarian.net/19906698/screen2.png



Comment 13 FiNeX 2009-02-20 20:10:10 UTC
*** Bug 185055 has been marked as a duplicate of this bug. ***
Comment 14 FiNeX 2009-02-20 20:11:10 UTC
I'm not able to reproduce it anymore using trunk and qt-copy. (r928825)
Comment 15 Frank Reininghaus 2009-02-20 23:41:00 UTC
(In reply to comment #14)
> I'm not able to reproduce it anymore using trunk and qt-copy. (r928825)

I can't confirm: trunk rev. 929171 with Qt 4.5.0-rc1 is still buggy for me :-(
Comment 16 Artem S. Tashkinov 2009-04-10 08:23:29 UTC
With Qt 4.5.0 and KDE 4.2.2 the bug is still there.
Comment 17 jem 2009-05-03 14:42:33 UTC
I can also confirm that the bug is still there on KDE 4.2.2 and Qt 4.5.1.
Comment 18 Frank Reininghaus 2009-06-13 11:44:52 UTC
*** Bug 196272 has been marked as a duplicate of this bug. ***
Comment 19 Viacheslav Tokarev 2009-06-23 18:53:41 UTC
As described in comment#9 the problem is with collapsed spaces in the rendering tree. But it's actually a general problem that code should be really clear when DOM or rendered offsets are needed.
As part of my SoC project (improved editing) I've addressed this issue and separated positions logic into khtml::RenderPosition and DOM::Position classes with rendering and DOM trees in mind.
The code will be merged a bit later in trunk (for 4.4).
Comment 20 Tommi Tervo 2009-08-23 16:25:37 UTC
*** Bug 204866 has been marked as a duplicate of this bug. ***
Comment 21 Frank Reininghaus 2009-11-25 15:40:14 UTC
*** Bug 216092 has been marked as a duplicate of this bug. ***
Comment 22 Frank Reininghaus 2009-11-25 18:39:35 UTC
It looks like text selection is broken in a different way in trunk: If you select a word using a double-click in the test case, the highlighting works OK (at least in the 2nd and 3rd line - in the 1st line, something still goes wrong), but the text that is actually copied to the clipboard is wrong (no matter how you do the selection, see bug 216092).
Comment 23 Germain Garand 2009-11-28 14:29:58 UTC
Created attachment 38649 [details]
convert Selection's DOM offsets to Rendered offsets, as this is what we need here.

(hmm, shouldn't Selection offer a direct interface to rendered offsets?
I fail to see in what case Selection is *not* about the Rendered string?)
Comment 24 Germain Garand 2009-11-30 06:06:26 UTC
SVN commit 1056420 by ggarand:

. convert the Selection's DOM offsets to Rendered offsets before using them
  on the rendered string (#166241, #216092)
. use slow font width algorithm when drawing selection for more accurate
  placement (#213246)

BUG: 166241
BUG: 216092
BUG: 213246

 M  +10 -7     khtml_part.cpp  
 M  +3 -3      rendering/render_text.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1056420