WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
85494
[EFL] Wrong button height on CSS tests
https://bugs.webkit.org/show_bug.cgi?id=85494
Summary
[EFL] Wrong button height on CSS tests
Thiago Marcos P. Santos
Reported
2012-05-03 07:08:02 PDT
fast/css/button-height.html with wrong results.
Attachments
Patch
(1.27 MB, patch)
2012-08-22 22:36 PDT
,
KyungTae Kim
no flags
Details
Formatted Diff
Diff
Patch
(1.27 MB, patch)
2012-08-26 18:52 PDT
,
KyungTae Kim
no flags
Details
Formatted Diff
Diff
Patch
(1.25 MB, patch)
2012-08-26 19:43 PDT
,
KyungTae Kim
no flags
Details
Formatted Diff
Diff
Patch
(1.25 MB, patch)
2012-08-26 22:28 PDT
,
KyungTae Kim
no flags
Details
Formatted Diff
Diff
Patch
(1.25 MB, patch)
2012-08-27 01:26 PDT
,
KyungTae Kim
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
KyungTae Kim
Comment 1
2012-08-22 22:36:52 PDT
Created
attachment 160087
[details]
Patch
KyungTae Kim
Comment 2
2012-08-22 22:37:25 PDT
***
Bug 91952
has been marked as a duplicate of this bug. ***
Gyuyoung Kim
Comment 3
2012-08-24 01:02:05 PDT
Comment on
attachment 160087
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=160087&action=review
> Source/WebCore/ChangeLog:8 > + The code for overriding styles originated from the GTK port's, but that was removed.
I don't understand this comment well. I understand EFL port's problematic code came from GTK port, but GTK port removed it. right? If so, you need to explain it more clearly.
> Source/WebCore/ChangeLog:9 > + (commit: 808a8f2a, bug:
https://bugs.webkit.org/show_bug.cgi?id=33936
)
I think revision number is more useful than commit#. For example,
r51455
instead of ae573116.
> Source/WebCore/ChangeLog:13 > + (commit for chromium port: ae573116, commit for layouttest: 14f37205)
ditto.
KyungTae Kim
Comment 4
2012-08-26 17:34:50 PDT
What I mean was only the EFL port overrides the Border,WhiteSpace,Height style now. The overriding codes seems coming from GTK port's old codes that are no longer exists. I'll explain why the overriding code must be removed, line by line : if (style->appearance() == PushButtonPart) { // Setting style only for <input type=button>, not <button> style->resetBorder(); // This code cause reset border style for button - 2px outset - to initial value. This code makes the size of <input type=button> different from the size of <button>, it makes 'fast/css/button-height.html' fail that check whether they are same. style->setWhiteSpace(PRE); // This code is not needed because the white space for buttons already set to 'PRE' style->setHeight(Length(Auto)); // This code override the style for height, it makes 'fast/css/button-height.html' fail that set the button's height style.
KyungTae Kim
Comment 5
2012-08-26 18:52:56 PDT
Created
attachment 160620
[details]
Patch
KyungTae Kim
Comment 6
2012-08-26 19:43:08 PDT
Created
attachment 160622
[details]
Patch
Gyuyoung Kim
Comment 7
2012-08-26 20:18:27 PDT
Comment on
attachment 160622
[details]
Patch I think adjustButtonStyle() function needs to be implemented according to port character. We don't need to reset some stuffs for "PushButtonPart" because Button size is already set by adjustSizeConstraints(). You're missing to unskip this test in TestExpectations. Please include TestExpectations file. By the way, don't you need to set initial value for height ? if (style->appearance() == PushButtonPart) { // Ignore line-height. style->setLineHeight(RenderStyle::initialLineHeight());
KyungTae Kim
Comment 8
2012-08-26 22:20:39 PDT
What you said is related with
https://bugs.webkit.org/show_bug.cgi?id=33936
. The code that initialize 'line-height' on the GTK and chromium ports is to fix the bug that bottom portion of text hangs off edge of SELECT or BUTTON element (layout test: fast/forms/control-restrict-line-height.html.) On the EFL port, because of "adjustSizeConstraints", the bug not occurs. Then, I think not to initialize 'line-height' is better to prevent bugs related with 'line-height' style on button.
KyungTae Kim
Comment 9
2012-08-26 22:28:34 PDT
Created
attachment 160634
[details]
Patch
Gyuyoung Kim
Comment 10
2012-08-26 22:33:03 PDT
Comment on
attachment 160634
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=160634&action=review
> LayoutTests/ChangeLog:62 > + * platform/efl/fast/replaced/width100percent-button-expected.txt:
In changelog, LayoutTests/platform/efl/TestExpectations modification is missing.
KyungTae Kim
Comment 11
2012-08-27 01:26:21 PDT
Created
attachment 160655
[details]
Patch
WebKit Review Bot
Comment 12
2012-08-27 05:40:00 PDT
Comment on
attachment 160655
[details]
Patch Clearing flags on attachment: 160655 Committed
r126750
: <
http://trac.webkit.org/changeset/126750
>
WebKit Review Bot
Comment 13
2012-08-27 05:40:05 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