NEW 129803
text selection highlight is visually incorrect under heavy padding constraints 
https://bugs.webkit.org/show_bug.cgi?id=129803
Summary text selection highlight is visually incorrect under heavy padding constraints
Mario Sanchez Prada
Reported 2014-03-06 08:05:02 PST
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
Attachments
Patch proposal (60.41 KB, patch)
2014-03-06 08:49 PST, Mario Sanchez Prada 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (578.26 KB, application/zip)
2014-03-06 09:56 PST, Build Bot 		no flags
Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (604.92 KB, application/zip)
2014-03-06 10:10 PST, Build Bot 		no flags
Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (605.84 KB, application/zip)
2014-03-06 11:10 PST, Build Bot 		no flags
Details
Patch proposal (64.01 KB, patch)
2014-03-07 02:11 PST, Mario Sanchez Prada 		rniwa: review-
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1 2014-03-06 08:49:29 PST
Created attachment 225994 [details] Patch proposal Patch proposal backporting the fix (and tests) from blink revision r167813
Build Bot
Comment 2 2014-03-06 09:55:59 PST
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
Build Bot
Comment 3 2014-03-06 09:56:02 PST
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
Build Bot
Comment 4 2014-03-06 10:10:01 PST
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
Build Bot
Comment 5 2014-03-06 10:10:05 PST
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
Build Bot
Comment 6 2014-03-06 11:10:01 PST
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
Build Bot
Comment 7 2014-03-06 11:10:04 PST
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
Mario Sanchez Prada
Comment 8 2014-03-07 02:11:59 PST
Created attachment 226101 [details] Patch proposal Considered output from EWS mac bots
Mario Sanchez Prada
Comment 9 2014-03-13 08:58:48 PDT
Ping reviewers?
Ryosuke Niwa
Comment 10 2014-03-13 14:49:19 PDT
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.
Mario Sanchez Prada
Comment 11 2014-03-14 06:34:12 PDT
(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?
Mario Sanchez Prada
Comment 12 2014-03-24 03:30:53 PDT
(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 :)
Note You need to log in before you can comment on or make changes to this bug.