This bug is tracks down an issue as reported for Blink as issue 313593[1]. Description of the issue as reported there is as follows: Steps to reproduce the problem: 1. Load the following HTML in WebKit: <html> <body> <div style="font-size: 16px; height: 20px; line-height: 0px;"> The House </div> <div style="font-size: 16px; height: 20px; line-height: 32px;"> The House </div> </body> </html> 2. Select all the text in each line separately (one line at a time) What is the expected behavior? You should see the selection rectangle properly drawn (not too small, not too big) when selecting "The House" in any of the two lines What went wrong? You see something like this for the two lines: - 1st line: http://goo.gl/4rgBCV - 2nd line: http://goo.gl/UToFkb [1] https://code.google.com/p/chromium/issues/detail?id=313593
Created attachment 225994 [details] Patch proposal Patch proposal backporting the fix (and tests) from blink revision r167813
Comment on attachment 225994 [details] Patch proposal Attachment 225994 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4607503778185216 New failing tests: fast/text/selection-rect-line-height-too-big.html editing/selection/inline-closest-leaf-child.html fast/text/selection-rect-line-height-too-small.html fast/inline-block/14498-positionForCoordinates.html
Created attachment 226003 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 225994 [details] Patch proposal Attachment 225994 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6134646777577472 New failing tests: fast/text/selection-rect-line-height-too-big.html editing/selection/inline-closest-leaf-child.html fast/text/selection-rect-line-height-too-small.html fast/inline-block/14498-positionForCoordinates.html
Created attachment 226006 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 225994 [details] Patch proposal Attachment 225994 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6702313141960704 New failing tests: fast/text/selection-rect-line-height-too-big.html editing/selection/inline-closest-leaf-child.html fast/text/selection-rect-line-height-too-small.html fast/inline-block/14498-positionForCoordinates.html
Created attachment 226013 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 226101 [details] Patch proposal Considered output from EWS mac bots
Ping reviewers?
Comment on attachment 226101 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=226101&action=review > LayoutTests/platform/mac/editing/selection/inline-closest-leaf-child-expected.txt:35 > -selection start: position 2 of child 0 {#text} of child 1 {SPAN} of child 4 {DIV} of body > -selection end: position 5 of child 2 {#text} of child 1 {SPAN} of child 4 {DIV} of body > +caret: position 0 of child 0 {#text} of child 1 {SPAN} of child 4 {DIV} of body Looks like this is a regression. r- due to the lack of description as to why new result is correct.
(In reply to comment #10) > (From update of attachment 226101 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226101&action=review > > > LayoutTests/platform/mac/editing/selection/inline-closest-leaf-child-expected.txt:35 > > -selection start: position 2 of child 0 {#text} of child 1 {SPAN} of child 4 {DIV} of body > > -selection end: position 5 of child 2 {#text} of child 1 {SPAN} of child 4 {DIV} of body > > +caret: position 0 of child 0 {#text} of child 1 {SPAN} of child 4 {DIV} of body > > Looks like this is a regression. r- due to the lack of description as to why new result is correct. You are right, sorry for not noticing it while uploading the patch. However, I think that such a regression is not actually caused by this patch, which seems to be unveiling the issue instead. I say this because I've manually loaded and experimented with the same test in Firefox and Blink (through a recently built version of chromium which includes this patch) and in both cases the behaviour is the expected one: I can select individual characters from "ipsum" by dragging the mouse in the yellow area below. However, if I try to do the same thing in WebKit, I get different results depending on whether this patch is applied or not (but both seem to be wrong): * Without the patch: I'm able to select characters by dragging in the yellow area above "ipsum", but the blue selection rectangle drawn is incorrect (height is too much, invading the yellow area). * With the patch: I can't select anything at all by dragging in the yellow area above "ipsum", which is also wrong. This does not happen in Blink with the same patch applied, though. As mentioned before, this backport patch addresses the first issue (selection rectangle incorrectly drawn) above. Unfortunately, it also unveils the second issue, which I believe is not related to this change since that's not happening in Blink after the very same changeset has been incorporated (I can select dragging in the yellow area && the selection rectangle is properly drawn, as in FF). So, perhaps the right thing to do here would be, instead of rebaselining those expectations (which I agree is wrong), to file a bug for the selection while dragging in the yellow area and add that test to the TestExpectations files. What do you think?
(In reply to comment #11) > [...] > So, perhaps the right thing to do here would be, instead of rebaselining those > expectations (which I agree is wrong), to file a bug for the selection while > dragging in the yellow area and add that test to the TestExpectations files. > > What do you think? It's been a while already since this comment. "Kindly" pinging then :)