As agreed in bug #121841 it seems a good idea to move common JavaScript functions related to CSS Regions selection tests to a common JavaScript file. fast/regions/selection/ tests will be refactored in order to use these new functions.
Created attachment 213039 [details] Patch
Comment on attachment 213039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213039&action=review Looks okay, some minor comments: > LayoutTests/fast/regions/resources/helper.js:206 > +function selectCoordinates(fromX, fromY, toX, toY) { I'd call this selectContentByRange. > LayoutTests/fast/regions/resources/helper.js:207 > + if (window.testRunner) { You should rather use early return to avoid unnecessary indentation. > LayoutTests/fast/regions/resources/helper.js:216 > +function selectIds(fromId, toId) { I'd prefer selectContentByIds. > LayoutTests/fast/regions/resources/helper.js:220 > + selectCoordinates(fromRect.left, fromRect.top + fromRect.height / 2, I'd make 2 temporary variables (fromRectVerticalCenter, ... ?) for the computations to make the code more readable. > LayoutTests/fast/regions/resources/helper.js:233 > + if (window.testRunner) { Just return early. Also, it seems it wasn't applied on the TOT.
Created attachment 213062 [details] Patch Thanks for the review, I've applied the changes. This doesn't apply in trunk because of it depends on bug #122099, which moves the CSS Regions layout tests related to selection to a specific folder "fast/regions/selection/".
Comment on attachment 213062 [details] Patch Cool! r=me
Comment on attachment 213062 [details] Patch Rejecting attachment 213062 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 213062, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: Tools/Scripts/update-webkit']" exit_code: 2 Updating OpenSource fatal: read error: Connection reset by peer Died at Tools/Scripts/update-webkit line 122. Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2 Updating OpenSource fatal: read error: Connection reset by peer Died at Tools/Scripts/update-webkit line 122. Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2 Updating OpenSource fatal: read error: Connection reset by peer Died at Tools/Scripts/update-webkit line 122. Full output: http://webkit-queues.appspot.com/results/3053007
Comment on attachment 213062 [details] Patch Set again cq+ due to some issues with commit-queue.
Comment on attachment 213062 [details] Patch Clearing flags on attachment: 213062 Committed r156807: <http://trac.webkit.org/changeset/156807>
All reviewed patches have been landed. Closing bug.