Bug 53520

Summary: Get rid of physical terminology like left/right/top/bottom on IntRect and FloatRect
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, buildbot, dglazkov, eric, gustavo.noronha, gustavo, ossy, tonikitoo, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 53613    
Bug Blocks:    
Attachments:
Description Flags
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
none
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
none
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
none
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
oliver: review-
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
oliver: review+
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
none
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
oliver: review+
Step 2: Remove the physical accessors.
darin: review+
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
none
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. hyatt: review+

Description Dave Hyatt 2011-02-01 12:30:01 PST
Remove the physical terminology from IntRect and FloatRect.  Now that we have flipped RenderBlocks for vertical-rl and horizontal-bt writing modes, we need to update our terminology to be more accurate.

I'm borrowing a page from AppKit here (which also supports flipped NSViews) and renaming right() and bottom() to maxX() and maxY().  These terms remain accurate even for flipped rectangles.
Comment 1 Dave Hyatt 2011-02-01 12:32:49 PST
Created attachment 80802 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 2 WebKit Review Bot 2011-02-01 12:34:54 PST
Attachment 80802 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/accessibility/Accessibility..." exit_code: 1

Source/WebCore/rendering/RenderTableCell.cpp:806:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 1 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Dave Hyatt 2011-02-01 12:36:02 PST
Created attachment 80803 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 4 Gustavo Noronha (kov) 2011-02-01 12:36:56 PST
Attachment 80802 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7684691
Comment 5 WebKit Review Bot 2011-02-01 12:38:24 PST
Attachment 80803 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/accessibility/Accessibility..." exit_code: 1

Source/WebCore/rendering/RenderTableCell.cpp:806:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 1 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Dave Hyatt 2011-02-01 12:43:18 PST
Created attachment 80804 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 7 WebKit Review Bot 2011-02-01 12:47:03 PST
Attachment 80802 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7681828
Comment 8 Gustavo Noronha (kov) 2011-02-01 12:47:39 PST
Attachment 80803 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7685655
Comment 9 Dave Hyatt 2011-02-01 12:49:24 PST
Created attachment 80807 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 10 WebKit Review Bot 2011-02-01 12:50:43 PST
Attachment 80803 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7684694
Comment 11 Oliver Hunt 2011-02-01 12:54:00 PST
Comment on attachment 80807 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.

r-, still has shiftMinYEdgeTo
Comment 12 Dave Hyatt 2011-02-01 12:54:53 PST
Created attachment 80808 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 13 Oliver Hunt 2011-02-01 12:56:57 PST
Comment on attachment 80808 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.

Add changelogs, and watch the build bots.  r=me
Comment 14 Early Warning System Bot 2011-02-01 12:59:13 PST
Attachment 80803 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7686565
Comment 15 WebKit Review Bot 2011-02-01 13:02:15 PST
Attachment 80804 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7682757
Comment 16 Collabora GTK+ EWS bot 2011-02-01 13:04:13 PST
Attachment 80804 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7680905
Comment 17 Gustavo Noronha (kov) 2011-02-01 13:10:38 PST
Attachment 80807 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7685665
Comment 18 Dave Hyatt 2011-02-01 13:12:58 PST
Created attachment 80812 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 19 WebKit Review Bot 2011-02-01 13:17:14 PST
Attachment 80808 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7680907
Comment 20 Build Bot 2011-02-01 13:18:01 PST
Attachment 80803 [details] did not build on win:
Build output: http://queues.webkit.org/results/7685668
Comment 21 Build Bot 2011-02-01 13:22:27 PST
Attachment 80802 [details] did not build on win:
Build output: http://queues.webkit.org/results/7684706
Comment 22 Build Bot 2011-02-01 13:23:47 PST
Attachment 80804 [details] did not build on win:
Build output: http://queues.webkit.org/results/7687290
Comment 23 Dave Hyatt 2011-02-01 13:25:06 PST
Created attachment 80813 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.
Comment 24 Oliver Hunt 2011-02-01 13:37:38 PST
Comment on attachment 80813 [details]
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet.

changelog! :)

r=me
Comment 25 Dave Hyatt 2011-02-01 13:40:04 PST
Step 1 landed in r77286.
Comment 26 Dave Hyatt 2011-02-01 13:41:25 PST
Created attachment 80819 [details]
Step 2: Remove the physical accessors.
Comment 27 WebKit Review Bot 2011-02-01 13:45:43 PST
Attachment 80812 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7685675
Comment 28 WebKit Review Bot 2011-02-01 13:48:48 PST
http://trac.webkit.org/changeset/77286 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Windows 32-bit Release
Comment 29 Darin Adler 2011-02-01 13:51:34 PST
Comment on attachment 80819 [details]
Step 2: Remove the physical accessors.

You should wait for the early warning system to tell you if you broke any ports before landing.
Comment 30 Gustavo Noronha (kov) 2011-02-01 13:56:03 PST
Attachment 80819 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7687301
Comment 31 WebKit Review Bot 2011-02-01 13:57:30 PST
Attachment 80813 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7687302
Comment 32 Dave Hyatt 2011-02-01 14:05:24 PST
Created attachment 80823 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 33 Collabora GTK+ EWS bot 2011-02-01 14:12:37 PST
Attachment 80823 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7684722
Comment 34 WebKit Review Bot 2011-02-01 14:13:03 PST
Attachment 80819 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7681858
Comment 35 WebKit Review Bot 2011-02-01 14:28:19 PST
Attachment 80823 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7680948
Comment 36 Dave Hyatt 2011-02-01 14:36:21 PST
Created attachment 80830 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 37 WebKit Review Bot 2011-02-01 15:06:18 PST
Attachment 80830 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7683744
Comment 38 Dave Hyatt 2011-02-01 15:20:41 PST
Created attachment 80837 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 39 WebKit Review Bot 2011-02-01 15:36:52 PST
Attachment 80837 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7686610
Comment 40 Build Bot 2011-02-01 15:53:04 PST
Attachment 80823 [details] did not build on win:
Build output: http://queues.webkit.org/results/7683761
Comment 41 Build Bot 2011-02-01 17:40:45 PST
Attachment 80837 [details] did not build on win:
Build output: http://queues.webkit.org/results/7683782
Comment 42 Gustavo Noronha (kov) 2011-02-01 17:52:55 PST
Attachment 80837 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7685725
Comment 43 Dave Hyatt 2011-02-01 18:00:36 PST
Created attachment 80864 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 44 Build Bot 2011-02-01 18:12:45 PST
Attachment 80837 [details] did not build on win:
Build output: http://queues.webkit.org/results/7686646
Comment 45 WebKit Review Bot 2011-02-01 18:16:20 PST
Attachment 80864 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7686650
Comment 46 Gustavo Noronha (kov) 2011-02-01 18:26:39 PST
Attachment 80864 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7689018
Comment 47 Build Bot 2011-02-01 19:22:31 PST
Attachment 80864 [details] did not build on win:
Build output: http://queues.webkit.org/results/7684808
Comment 48 Dave Hyatt 2011-02-01 20:21:52 PST
Created attachment 80878 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 49 WebKit Review Bot 2011-02-01 20:37:14 PST
Attachment 80878 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7683835
Comment 50 Dave Hyatt 2011-02-01 20:40:12 PST
Created attachment 80880 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 51 WebKit Review Bot 2011-02-01 20:55:39 PST
Attachment 80880 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7682885
Comment 52 Build Bot 2011-02-01 21:18:14 PST
Attachment 80878 [details] did not build on win:
Build output: http://queues.webkit.org/results/7685776
Comment 53 Build Bot 2011-02-01 21:27:24 PST
Attachment 80880 [details] did not build on win:
Build output: http://queues.webkit.org/results/7685778
Comment 54 Csaba Osztrogonác 2011-02-01 23:29:05 PST
http://trac.webkit.org/changeset/77328(In reply to comment #25)
> Step 1 landed in r77286.

Qt buildfix: http://trac.webkit.org/changeset/77291 (by Hyatt)
Chromium buildfix: http://trac.webkit.org/changeset/77292 (by Hyatt)
Qt buildfix: http://trac.webkit.org/changeset/77300 (by Hyatt)
Qt buildfix: http://trac.webkit.org/changeset/77312 (by Hyatt)
Qt buildfix: http://trac.webkit.org/changeset/77316 (by Hyatt)
Qt buildfix: http://trac.webkit.org/changeset/77317 (by Hyatt)
Qt buildfix: http://trac.webkit.org/changeset/77328 (by me)

If you had asked a Qt guy, it would have been easiear ...
Comment 55 Dave Hyatt 2011-02-02 09:29:24 PST
Created attachment 80922 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 56 Dave Hyatt 2011-02-02 10:34:38 PST
Created attachment 80929 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 57 Dave Hyatt 2011-02-02 10:36:09 PST
Created attachment 80930 [details]
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors.
Comment 58 WebKit Review Bot 2011-02-02 10:56:04 PST
Attachment 80929 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7685941
Comment 59 WebKit Review Bot 2011-02-02 10:59:10 PST
Attachment 80930 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7690139
Comment 60 Early Warning System Bot 2011-02-02 11:29:51 PST
Attachment 80930 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7691050
Comment 61 Dave Hyatt 2011-02-02 12:21:06 PST
Fixed in r77398.
Comment 62 WebKit Review Bot 2011-02-02 15:46:41 PST
http://trac.webkit.org/changeset/77405 might have broken GTK Linux 32-bit Release
The following tests are not passing:
editing/selection/find-yensign-and-backslash.html
http/tests/websocket/tests/handshake-fail-by-sub-protocol-mismatch.html