| Summary: | ASSERTION FAILED with -webkit-appearance:progress-bar for non-progress bar elements | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||||||
| Component: | Forms | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | abarth, dglazkov, eric, tkent, webkit.review.bot, yael | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | OS X 10.5 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
Created attachment 57851 [details]
patch v0
Attachment 57851 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3090020 Attachment 57851 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3138007 Created attachment 57855 [details]
patch v1; attempt to chromium build fix
Comment on attachment 57855 [details]
patch v1; attempt to chromium build fix
I missed Qt build fix. canceling.
Comment on attachment 57855 [details]
patch v1; attempt to chromium build fix
Oops. Qt build failure has same cause as Chromium's one. re-enable this patch...
Attachment 57851 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3133018 Comment on attachment 57855 [details]
patch v1; attempt to chromium build fix
LayoutTests/fast/css/script-tests/invalid-appearance-progress-bar-meter.js:3
+ var body = document.getElementsByTagName("body")[0];
document.body is simpler.
WebCore/rendering/RenderTheme.cpp:47
+ #import "RenderProgress.h"
Why #import instead of #include ?
WebCore/rendering/RenderTheme.h:245
+ virtual bool paintMeter(RenderMeter*, const RenderObject::PaintInfo&, const IntRect&);
For consistency with other paint*() member functions, I prefer not changing the parameter types, and check isMeter() and isProgress() in their paint*() functions.
Created attachment 57859 [details]
patch v2
Attachment 57859 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/rendering/RenderThemeChromiumSkia.cpp:805: Code inside a namespace should not be indented. [whitespace/indent] [4]
Total errors found: 1 in 10 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 57862 [details]
patch v3; rebased
Hi Kent-san, Thank you for your quick review! > (From update of attachment 57855 [details]) > LayoutTests/fast/css/script-tests/invalid-appearance-progress-bar-meter.js:3 > + var body = document.getElementsByTagName("body")[0]; > document.body is simpler. Done. > > > WebCore/rendering/RenderTheme.cpp:47 > + #import "RenderProgress.h" > Why #import instead of #include ? Just copied from .mm file and didn't noticed. I'm sorry for confusion. > WebCore/rendering/RenderTheme.h:245 > + virtual bool paintMeter(RenderMeter*, const RenderObject::PaintInfo&, const IntRect&); > For consistency with other paint*() member functions, I prefer not changing the parameter types, and check isMeter() and isProgress() in their paint*() functions. Done. Attachment 57859 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3107026 Comment on attachment 57862 [details]
patch v3; rebased
WebCore/platform/qt/RenderThemeQt.cpp:731
+ return false;
paint*() function should return true if it doesn't paint something.
Created attachment 57988 [details]
patch v4
Thank you for reviewing!
> WebCore/platform/qt/RenderThemeQt.cpp:731
> + return false;
> paint*() function should return true if it doesn't paint something.
Fixed to return true. I also fixed other similar parts as well.
Comment on attachment 57988 [details]
patch v4
OK
Committed r60767: <http://trac.webkit.org/changeset/60767> http://trac.webkit.org/changeset/60767 might have broken Chromium Win Release Comment on attachment 57988 [details]
patch v4
clearing r+
|
Created attachment 57847 [details] reproduce ASSERTION FAILED occurs when "-webkit-appearance:progress-bar" is given for non-progress element like <input style="-webkit-appearance: progress-bar;" />. <meter> should have same problem, but it is not enabled yet.