NEW 55341
Add lineboundary editing/selection/extend-selection test
https://bugs.webkit.org/show_bug.cgi?id=55341
Summary Add lineboundary editing/selection/extend-selection test
Benjamin (Ben) Kalman
Reported 2011-02-27 18:56:56 PST
Add lineboundary editing/selection/extend-selection test
Attachments
Patch (19.07 KB, patch)
2011-02-27 18:58 PST, Benjamin (Ben) Kalman
no flags
Patch (41.33 KB, patch)
2011-02-27 19:49 PST, Benjamin (Ben) Kalman
no flags
Patch (40.75 KB, patch)
2011-02-27 20:04 PST, Benjamin (Ben) Kalman
ojan: review-
Benjamin (Ben) Kalman
Comment 1 2011-02-27 18:58:21 PST
Benjamin (Ben) Kalman
Comment 2 2011-02-27 18:58:59 PST
I think this test might actually obsolete editing/selection/extend-selection-home-end
Ryosuke Niwa
Comment 3 2011-02-27 19:36:54 PST
(In reply to comment #2) > I think this test might actually obsolete editing/selection/extend-selection-home-end It definitely does. Please modify extend-selection-home-end.html instead of adding new test that's a superset of the old test.
Ryosuke Niwa
Comment 4 2011-02-27 19:37:27 PST
Comment on attachment 84008 [details] Patch r- per my comment above.
Benjamin (Ben) Kalman
Comment 5 2011-02-27 19:49:51 PST
Benjamin (Ben) Kalman
Comment 6 2011-02-27 19:53:02 PST
home-end is badly named, it should be lineboundary. Consider it an obsoletion rather than a modification.
Ryosuke Niwa
Comment 7 2011-02-27 20:01:02 PST
Comment on attachment 84010 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84010&action=review (In reply to comment #6) > home-end is badly named, it should be lineboundary. > > Consider it an obsoletion rather than a modification. I'm not sure. home-end signifies the fact we're testing the behavior of home and end keys wheres line-boundary doesn't. I don't think new name is significantly better. In general, I'm hesitant to rename tests unless there's a really compelling reason to do so. For example, flakiness dashboard will lose all test results if we did this rename. > LayoutTests/ChangeLog:32 > +2011-02-27 Benjamin Kalman <kalman@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Add lineboundary editing/selection/extend-selection test > + https://bugs.webkit.org/show_bug.cgi?id=55341 > + > + This is the same as most other extend-selection tests where everything is done in the script test. > + A lineboundary version of the test didn't initially exist due to RTL bugs, which have now been fixed. > + > + * editing/selection/extend-selection-lineboundary-expected.txt: Added. > + * editing/selection/extend-selection-lineboundary.html: Added. > + Nit: old change log.
Benjamin (Ben) Kalman
Comment 8 2011-02-27 20:03:32 PST
Well, lineboundary is actually a disjoint set from home-end. Home-end is platform dependent, lineboundary isn't. So actually they're quite distinct.
Benjamin (Ben) Kalman
Comment 9 2011-02-27 20:04:25 PST
Ryosuke Niwa
Comment 10 2011-02-27 20:09:44 PST
Comment on attachment 84011 [details] Patch (In reply to comment #8) > Well, lineboundary is actually a disjoint set from home-end. Home-end is platform dependent, lineboundary isn't. So actually they're quite distinct. Ok, but I'm still skeptical of the benefit of this rename. I'm going to wait until some other reviewer can comment on the benefit of renaming this test.
Ojan Vafai
Comment 11 2011-02-27 20:16:51 PST
(In reply to comment #10) > (From update of attachment 84011 [details]) > (In reply to comment #8) > > Well, lineboundary is actually a disjoint set from home-end. Home-end is platform dependent, lineboundary isn't. So actually they're quite distinct. > > Ok, but I'm still skeptical of the benefit of this rename. I'm going to wait until some other reviewer can comment on the benefit of renaming this test. home-end is a poor name for this test since it actually is only testing lineboundary behavior. The new name is clearly more accurate. But the rename is orthogonal to cleaning up the test to using runSelectionTestsWithGranularity(createAllNodes(), "lineBoundary"). Ben, would you mind making them in two separate patches? Also, as you noted in person, you'll need to use SVN to do the move properly since "git svn" doesn't handle it correctly.
Ojan Vafai
Comment 12 2011-02-27 20:17:10 PST
Comment on attachment 84011 [details] Patch r- per comment 11.
Benjamin (Ben) Kalman
Comment 13 2011-02-27 20:52:28 PST
(In reply to comment #8) > Well, lineboundary is actually a disjoint set from home-end. Home-end is platform dependent, lineboundary isn't. So actually they're quite distinct. I'm wrong about this, sorry. Extend by lineboundary is platform dependent, though home-end is more platform divergent. That said, as Ojan mentioned, this test isn't testing home-end but rather lineboundary anyway.
Note You need to log in before you can comment on or make changes to this bug.