[Webkit-unassigned] [Bug 33413] selection.modify extends too far with 'lineboundary'

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 10 10:26:13 PST 2010


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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50376|review?                     |review-
               Flag|                            |




--- Comment #6 from Alexey Proskuryakov <ap at webkit.org>  2010-03-10 10:26:12 PST ---
(From update of attachment 50376)
+             * So we mimic UPSTREAM affinity to prevent to crossing
+             * line when the offset is at the end of line.

Two comments:
1) We use C++ style comments ("//") in C++ code.
2) I don't understand why this says "mimic". Don't we just plain use it? And
would anything break if you didn't add
"pos.setAffinity(m_selection.affinity());"?

> Or, you can leave the change description minimal and annotate the files/methods instead.

Yes, that's the preferred way of writing ChangeLogs.

> +        Re-baseline: editing/selection/extend-selection-expected.txt

"Baseline" is a noun, so using it as a verb is a mistake. You could just say
"updated results" instead.

> I'm not really sure what sentenceboundary/paragraphboundary are used for, 
> i.e. do they correspond to a user-action?

Without actually checking the code, I suspect that you get paragraphboundary
when triple-clicking and dragging, and also when triple-clicking and then
extending the selection from keyboard with Cmd+Shift+Right (but when I tested,
the results from the latter seemed wrong).

I think this needs another iteration, r- for now. I agree with Ojan's comments
about code changes.

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



More information about the webkit-unassigned mailing list