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 Rendering

Assignee:

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:

Version:

528+ (Nightly build)

Hardware:

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+

Dave Hyatt

Reported 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.

Attachments
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (105.52 KB, patch) 2011-02-01 12:32 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (105.52 KB, patch) 2011-02-01 12:36 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (104.89 KB, patch) 2011-02-01 12:43 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (105.58 KB, patch) 2011-02-01 12:49 PST, Dave Hyatt	oliver: review-	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (105.57 KB, patch) 2011-02-01 12:54 PST, Dave Hyatt	oliver: review+	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (110.12 KB, patch) 2011-02-01 13:12 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 1: Convert most of WebCore and WebKit without getting rid of the methods yet. (111.39 KB, patch) 2011-02-01 13:25 PST, Dave Hyatt	oliver: review+	Details Formatted Diff Diff
Step 2: Remove the physical accessors. (1.60 KB, patch) 2011-02-01 13:41 PST, Dave Hyatt	darin: review+	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (7.27 KB, patch) 2011-02-01 14:05 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (11.55 KB, patch) 2011-02-01 14:36 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (16.77 KB, patch) 2011-02-01 15:20 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (34.28 KB, patch) 2011-02-01 18:00 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (39.76 KB, patch) 2011-02-01 20:21 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (41.00 KB, patch) 2011-02-01 20:40 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (44.92 KB, patch) 2011-02-02 09:29 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (3.14 KB, patch) 2011-02-02 10:34 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Step 2: (Reviewed by Darin, just using bots to check for errors) Remove accessors. (1.60 KB, patch) 2011-02-02 10:36 PST, Dave Hyatt	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (16) View All Add attachment proposed patch, testcase, etc.

Dave Hyatt

Comment 1 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.

WebKit Review Bot

Comment 2 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.

Dave Hyatt

Comment 3 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.

Gustavo Noronha (kov)

Comment 4 2011-02-01 12:36:56 PST

Attachment 80802 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7684691

WebKit Review Bot

Comment 5 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.

Dave Hyatt

Comment 6 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.

WebKit Review Bot

Comment 7 2011-02-01 12:47:03 PST

Attachment 80802 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7681828

Gustavo Noronha (kov)

Comment 8 2011-02-01 12:47:39 PST

Attachment 80803 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7685655

Dave Hyatt

Comment 9 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.

WebKit Review Bot

Comment 10 2011-02-01 12:50:43 PST

Attachment 80803 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7684694

Oliver Hunt

Comment 11 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

Dave Hyatt

Comment 12 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.

Oliver Hunt

Comment 13 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

Early Warning System Bot

Comment 14 2011-02-01 12:59:13 PST

Attachment 80803 [details] did not build on qt: Build output: http://queues.webkit.org/results/7686565

WebKit Review Bot

Comment 15 2011-02-01 13:02:15 PST

Attachment 80804 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7682757

Collabora GTK+ EWS bot

Comment 16 2011-02-01 13:04:13 PST

Attachment 80804 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7680905

Gustavo Noronha (kov)

Comment 17 2011-02-01 13:10:38 PST

Attachment 80807 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7685665

Dave Hyatt

Comment 18 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.

WebKit Review Bot

Comment 19 2011-02-01 13:17:14 PST

Attachment 80808 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7680907

Build Bot

Comment 20 2011-02-01 13:18:01 PST

Attachment 80803 [details] did not build on win: Build output: http://queues.webkit.org/results/7685668

Build Bot

Comment 21 2011-02-01 13:22:27 PST

Attachment 80802 [details] did not build on win: Build output: http://queues.webkit.org/results/7684706

Build Bot

Comment 22 2011-02-01 13:23:47 PST

Attachment 80804 [details] did not build on win: Build output: http://queues.webkit.org/results/7687290

Dave Hyatt

Comment 23 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.

Oliver Hunt

Comment 24 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

Dave Hyatt

Comment 25 2011-02-01 13:40:04 PST

Step 1 landed in r77286.

Dave Hyatt

Comment 26 2011-02-01 13:41:25 PST

Created attachment 80819 [details] Step 2: Remove the physical accessors.

WebKit Review Bot

Comment 27 2011-02-01 13:45:43 PST

Attachment 80812 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7685675

WebKit Review Bot

Comment 28 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

Darin Adler

Comment 29 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.

Gustavo Noronha (kov)

Comment 30 2011-02-01 13:56:03 PST

Attachment 80819 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7687301

WebKit Review Bot

Comment 31 2011-02-01 13:57:30 PST

Attachment 80813 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7687302

Dave Hyatt

Comment 32 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.

Collabora GTK+ EWS bot

Comment 33 2011-02-01 14:12:37 PST

Attachment 80823 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7684722

WebKit Review Bot

Comment 34 2011-02-01 14:13:03 PST

Attachment 80819 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7681858

WebKit Review Bot

Comment 35 2011-02-01 14:28:19 PST

Attachment 80823 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7680948

Dave Hyatt

Comment 36 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.

WebKit Review Bot

Comment 37 2011-02-01 15:06:18 PST

Attachment 80830 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7683744

Dave Hyatt

Comment 38 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.

WebKit Review Bot

Comment 39 2011-02-01 15:36:52 PST

Attachment 80837 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7686610

Build Bot

Comment 40 2011-02-01 15:53:04 PST

Attachment 80823 [details] did not build on win: Build output: http://queues.webkit.org/results/7683761

Build Bot

Comment 41 2011-02-01 17:40:45 PST

Attachment 80837 [details] did not build on win: Build output: http://queues.webkit.org/results/7683782

Gustavo Noronha (kov)

Comment 42 2011-02-01 17:52:55 PST

Attachment 80837 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7685725

Dave Hyatt

Comment 43 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.

Build Bot

Comment 44 2011-02-01 18:12:45 PST

Attachment 80837 [details] did not build on win: Build output: http://queues.webkit.org/results/7686646

WebKit Review Bot

Comment 45 2011-02-01 18:16:20 PST

Attachment 80864 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7686650

Gustavo Noronha (kov)

Comment 46 2011-02-01 18:26:39 PST

Attachment 80864 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7689018

Build Bot

Comment 47 2011-02-01 19:22:31 PST

Attachment 80864 [details] did not build on win: Build output: http://queues.webkit.org/results/7684808

Dave Hyatt

Comment 48 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.

WebKit Review Bot

Comment 49 2011-02-01 20:37:14 PST

Attachment 80878 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7683835

Dave Hyatt

Comment 50 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.

WebKit Review Bot

Comment 51 2011-02-01 20:55:39 PST

Attachment 80880 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7682885

Build Bot

Comment 52 2011-02-01 21:18:14 PST

Attachment 80878 [details] did not build on win: Build output: http://queues.webkit.org/results/7685776

Build Bot

Comment 53 2011-02-01 21:27:24 PST

Attachment 80880 [details] did not build on win: Build output: http://queues.webkit.org/results/7685778

Csaba Osztrogonác

Comment 54 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 ...

Dave Hyatt

Comment 55 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.

Dave Hyatt

Comment 56 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.

Dave Hyatt

Comment 57 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.

WebKit Review Bot

Comment 58 2011-02-02 10:56:04 PST

Attachment 80929 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7685941

WebKit Review Bot

Comment 59 2011-02-02 10:59:10 PST

Attachment 80930 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7690139

Early Warning System Bot

Comment 60 2011-02-02 11:29:51 PST

Attachment 80930 [details] did not build on qt: Build output: http://queues.webkit.org/results/7691050

Dave Hyatt

Comment 61 2011-02-02 12:21:06 PST

Fixed in r77398.

WebKit Review Bot

Comment 62 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

Note You need to log in before you can comment on or make changes to this bug.