[Webkit-unassigned] [Bug 36256] shift+click to extend a wordgranularity selection backwards selects the space after

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36256





--- Comment #16 from Van Lam <vanlam at google.com>  2011-09-15 14:48:00 PST ---
(In reply to comment #15)
> (From update of attachment 104955 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=104955&action=review
> 
> >>>>>>>>> Source/WebCore/editing/VisibleSelection.cpp:300
> >>>>>>>>> +            if (isEndOfDocument(originalEnd) || (isEndOfLine(originalEnd) && !isStartOfLine(originalEnd) && !isEndOfParagraph(originalEnd)) || (isEndOfWord(originalEnd) && start != originalEnd && m_extent != originalEnd))
> >>>>>>>> 
> >>>>>>>> I'd split it into two lines since this is really long line.  ap & mitz should probably look at this change.
> >>>>>>> 
> >>>>>>> No, if c is at the end of a word, endOfWord(c, RightWordIfOnBoundary) returns the end of the following word.
> >>>>>> 
> >>>>>> 
> >>>>> 
> >>>>> Given that I'm checking isEndOfWord(originalEnd) && start != originalEnd, without doing the additional m_extent != originalEnd check, word-granularity mouse selections don't work correctly. What happens is given:
> >>>>> 
> >>>>> foo bar baz
> >>>>> 
> >>>>> If you double click in 'bar' and drag the mouse to the right to the position after the r in 'bar, it should grow the selection to include the space after 'bar' (in general the next word); it doesn't grow the selection if I omit the check in question.
> >>>>> 
> >>>>> I do admit it's not very clear what this check is doing; I'll try to find a more clear way to do this check. I initially saw that the first check (if originalEnd is at the end of a word) would fix the problem and added the other two checks to keep other behavior the same.
> >>>> 
> >>>> Shouldn't we be comparing it with m_end then?
> >>> 
> >>> originalEnd is declared:
> >>> 
> >>> VisiblePosition originalEnd(m_end, m_affinity); and is not modified until my check, so there shouldn't be a difference.
> >> 
> >> But you see
> >>     if (m_baseIsFirst) {
> >>         m_start = m_base;
> >>         m_end = m_extent;
> >>     } else {
> >>         m_start = m_extent;
> >>         m_end = m_base;
> >>     }
> >> above, right?
> > 
> > Wouldn't we need to depend on whether or not the base is first? Because the correct behavior from what I understand is that when dragging by word-granularity to the end of a word, the next word gets selected; on the other hand, when dragging by word-granularity to the start of a word, the previous word doesn't get selected. Let me know if I misunderstood your point.
> > 
> > But I don't see a difference between comparing m_extent to m_end and comparing m_extent to originalEnd in this context.
> 
> It seems like your assumption breaks down on Mac since selection is directionless on Mac.

I'm not sure how that breaks down my assumption (what is my assumption exactly?). Mac selections while directionless are still implemented using base and extent such that the greater of the two is used to determine the end of the selection.

But understanding that this proposed fix produces the correct behavior while keeping other behavior the same (no test failures in editing/selection/), do you have any suggestions on how it can be further improved?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.