WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
68968
Only resolve direction aware properties when properties are direction aware.
https://bugs.webkit.org/show_bug.cgi?id=68968
Summary
Only resolve direction aware properties when properties are direction aware.
Luke Macpherson
Reported
2011-09-27 21:02:06 PDT
Only resolve direction aware properties when properties are direction aware.
Attachments
Patch
(2.44 KB, patch)
2011-09-27 21:05 PDT
,
Luke Macpherson
no flags
Details
Formatted Diff
Diff
Patch
(2.52 KB, patch)
2011-09-27 23:33 PDT
,
Luke Macpherson
no flags
Details
Formatted Diff
Diff
Patch for landing
(2.13 KB, patch)
2011-09-28 16:45 PDT
,
Luke Macpherson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Luke Macpherson
Comment 1
2011-09-27 21:05:20 PDT
Created
attachment 108957
[details]
Patch
Eric Seidel (no email)
Comment 2
2011-09-27 22:28:57 PDT
I'm not entirely sure what this does. How will we know when we get this wrong? How do we know that we are using the correct list of "directionally aware" properties?
Luke Macpherson
Comment 3
2011-09-27 23:09:20 PDT
(In reply to
comment #2
)
> I'm not entirely sure what this does. How will we know when we get this wrong? How do we know that we are using the correct list of "directionally aware" properties?
At the moment we try to resolve all property id's which is unnecessary. We can't really miss any because all properties are listed in the switch in applyProperty().
Eric Seidel (no email)
Comment 4
2011-09-27 23:13:29 PDT
Comment on
attachment 108957
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=108957&action=review
This seems reasonable to make directional properties slower at the cost of making all other properties faster.
> Source/WebCore/css/CSSStyleSelector.cpp:3635 > + return applyProperty(CSSProperty::resolveDirectionAwareProperty(id, m_style->direction(), m_style->writingMode()), value);
Are we sure we're calling this with the right arguments?
Luke Macpherson
Comment 5
2011-09-27 23:19:10 PDT
Comment on
attachment 108957
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=108957&action=review
>> Source/WebCore/css/CSSStyleSelector.cpp:3635 >> + return applyProperty(CSSProperty::resolveDirectionAwareProperty(id, m_style->direction(), m_style->writingMode()), value); > > Are we sure we're calling this with the right arguments?
Looks ok to me - resolveDirectionAwareProperty just translates the direction aware property id to the appropriate property id for the current context. I might add an ASSERT_NOT_REACHED() to the default case of that function, just to ensure we can't infinite loop here.
Luke Macpherson
Comment 6
2011-09-27 23:33:09 PDT
Created
attachment 108971
[details]
Patch
Eric Seidel (no email)
Comment 7
2011-09-27 23:40:29 PDT
Comment on
attachment 108971
[details]
Patch This is much clearer now, thank you.
WebKit Review Bot
Comment 8
2011-09-28 15:29:34 PDT
Comment on
attachment 108971
[details]
Patch Rejecting
attachment 108971
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: sed 2 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/css/CSSStyleSelector.cpp Hunk #1 FAILED at 2360. Hunk #2 succeeded at 3596 (offset -2 lines). Hunk #3 succeeded at 3632 (offset -2 lines). 1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/css/CSSStyleSelector.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output:
http://queues.webkit.org/results/9881821
Luke Macpherson
Comment 9
2011-09-28 16:45:07 PDT
Created
attachment 109090
[details]
Patch for landing
WebKit Review Bot
Comment 10
2011-09-28 18:00:20 PDT
Comment on
attachment 109090
[details]
Patch for landing Clearing flags on attachment: 109090 Committed
r96281
: <
http://trac.webkit.org/changeset/96281
>
WebKit Review Bot
Comment 11
2011-09-28 18:00:25 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug