WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
73916
Drop ENABLE_NO_LISTBOX_RENDERING, and make it a runtime decision.
https://bugs.webkit.org/show_bug.cgi?id=73916
Summary
Drop ENABLE_NO_LISTBOX_RENDERING, and make it a runtime decision.
Pierre Rossi
Reported
2011-12-06 05:55:37 PST
Drop ENABLE_NO_LISTBOX_RENDERING, and make it a runtime decision.
Attachments
Patch
(14.54 KB, patch)
2011-12-06 06:42 PST
,
Pierre Rossi
no flags
Details
Formatted Diff
Diff
Patch
(14.55 KB, patch)
2011-12-07 06:18 PST
,
Pierre Rossi
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Pierre Rossi
Comment 1
2011-12-06 06:42:44 PST
Created
attachment 118040
[details]
Patch
Kent Tamura
Comment 2
2011-12-06 18:13:10 PST
Comment on
attachment 118040
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=118040&action=review
Thank you for working on this!
> Source/WebCore/html/HTMLSelectElement.cpp:187 > + Page* page = 0; > + if (Frame* frame = document()->frame()) > + page = frame->page();
We have Document::page().
> Source/WebCore/html/HTMLSelectElement.cpp:188 > + RenderTheme* renderTheme = page ? page->theme() : RenderTheme::defaultTheme().leakRef();
Do not leak. This should be RefPtr<RenterTheme> renderTheme = ...
> Source/WebCore/rendering/RenderTheme.h:210 > + virtual bool usesMenuList() const { return false; }
The function name should be more descriptive. selectElementAlwaysUsesMenuList() or something?
Pierre Rossi
Comment 3
2011-12-07 06:18:21 PST
Created
attachment 118203
[details]
Patch @Kent: thanks for taking a look at this. Any idea what render themes in chromium would need to switch to this ? I failed to track the define all the way.
Kent Tamura
Comment 4
2011-12-07 19:26:58 PST
(In reply to
comment #3
)
> @Kent: thanks for taking a look at this. Any idea what render themes in chromium would need to switch to this ? I failed to track the define all the way.
I think Android port is using ENABLE_NO_LISTBOX_RENDERING, but RenderTheme for Android is not upstreamed yet. So you don't need to care about it.
Kent Tamura
Comment 5
2011-12-07 19:27:35 PST
Comment on
attachment 118203
[details]
Patch Looks good.
WebKit Review Bot
Comment 6
2011-12-08 04:54:49 PST
Comment on
attachment 118203
[details]
Patch Rejecting
attachment 118203
[details]
from commit-queue.
pierre.rossi@gmail.com
does not have committer permissions according to
http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py
. - If you do not have committer rights please read
http://webkit.org/coding/contributing.html
for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
WebKit Review Bot
Comment 7
2011-12-08 19:22:35 PST
Comment on
attachment 118203
[details]
Patch Clearing flags on attachment: 118203 Committed
r102419
: <
http://trac.webkit.org/changeset/102419
>
WebKit Review Bot
Comment 8
2011-12-08 19:22:40 PST
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