53906 2011-02-06 23:45:11 -0800 [EFL] Add dummy functions for HTML5 Video's control UI 2011-02-09 03:54:39 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 gyuyoung.kim webkit-unassigned commit-queue eric kenneth leandro lucas.de.marchi tonikitoo oldest_to_newest 346489 0 gyuyoung.kim 2011-02-06 23:45:11 -0800 In WebKit EFL, there are no implementations for HTML5 control UI. I am implementing the functions. First of all, I'd like to add dummy functions for them. 346492 1 81453 gyuyoung.kim 2011-02-06 23:48:29 -0800 Created attachment 81453 Patch 346890 2 81453 eric 2011-02-07 15:22:19 -0800 Comment on attachment 81453 Patch OK. So currently the compile fails if you enable VIDEO w/o these? Or does it just use the defaults? if it uses the default implementations then it seems these are implemented incorrectly (and should instead call the parent function. 346906 3 gyuyoung.kim 2011-02-07 15:50:49 -0800 (In reply to comment #2) > (From update of attachment 81453 [details]) > OK. So currently the compile fails if you enable VIDEO w/o these? Or does it just use the defaults? if it uses the default implementations then it seems these are implemented incorrectly (and should instead call the parent function. In WebKit EFL, VIDEO feature is enabled by default. So, these functions will be used by default. Currently, these functions which paint players's control UI are defined in RenderTheme.h file virtually. 283 virtual bool paintMediaFullscreenButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 284 virtual bool paintMediaPlayButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 285 virtual bool paintMediaMuteButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 286 virtual bool paintMediaSeekBackButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 287 virtual bool paintMediaSeekForwardButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 288 virtual bool paintMediaSliderTrack(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 289 virtual bool paintMediaSliderThumb(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 290 virtual bool paintMediaVolumeSliderContainer(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 291 virtual bool paintMediaVolumeSliderTrack(RenderObject*, const ... Now, the parent functions just return "true" because WebKit EFL doesn't implement these functions yet. So, first of all, I add dummy functions, then I'd like to implement these functions step by step. If I have misunderstanding, please let me know. Thank you. :-) 347086 4 81453 eric 2011-02-07 21:57:47 -0800 Comment on attachment 81453 Patch And remind me what the returning false vs. true means? (Yes, we really really really need to change that to be an enum). 347842 5 gyuyoung.kim 2011-02-08 17:29:13 -0800 (In reply to comment #4) > (From update of attachment 81453 [details]) > And remind me what the returning false vs. true means? (Yes, we really really really need to change that to be an enum). paintMediaXXXX functions are invoked by below function. bool RenderTheme::paint(RenderObject* o, const PaintInfo& paintInfo, const IntRect& r) AFAIK, this paint function draws media control UI. And, the paint() is called by RenderBox::paintBoxDecorationsWithSize() as below, void RenderBox::paintBoxDecorationsWithSize(PaintInfo& paintInfo, int tx, int ty, int width, int height) { ... // If we have a native theme appearance, paint that before painting our background. // The theme will tell us whether or not we should also paint the CSS background. bool themePainted = style()->hasAppearance() && !theme()->paint(this, paintInfo, IntRect(tx, ty, width, height)); => This line calls the paint function. if (!themePainted) { // The <body> only paints its background if the root element has defined a background // independent of the body. Go through the DOM to get to the root element's render object, // since the root could be inline and wrapped in an anonymous block. if (!isBody() || document()->documentElement()->renderer()->hasBackground()) paintFillLayers(paintInfo, style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->backgroundLayers(), tx, ty, width, height); if (style()->hasAppearance()) theme()->paintDecorations(this, paintInfo, IntRect(tx, ty, width, height)); } It seems to me that the "true" or "false" return value mean whether each port paints UI theme or not. Thus, I think this is correct that EFL port returns "false". Because, it doesn't paint nothing until implementing these dummy functions. If we need to change the boolean return value with enum type, we should modify RenderBox::paintBoxDecorationWithSize and each port's paint functions together. Do you think we should modify now ? 348075 6 eric 2011-02-09 02:00:15 -0800 (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 81453 [details] [details]) > If we need to change the boolean return value with enum type, we should modify RenderBox::paintBoxDecorationWithSize and each port's paint functions together. Do you think we should modify now ? Nah. That's a separate patch. 348077 7 81453 eric 2011-02-09 02:01:32 -0800 Comment on attachment 81453 Patch OK. 348137 8 81453 commit-queue 2011-02-09 03:54:34 -0800 Comment on attachment 81453 Patch Clearing flags on attachment: 81453 Committed r78041: <http://trac.webkit.org/changeset/78041> 348138 9 commit-queue 2011-02-09 03:54:39 -0800 All reviewed patches have been landed. Closing bug. 81453 2011-02-06 23:48:29 -0800 2011-02-09 03:54:34 -0800 Patch patch-html5control.patch text/plain 5628 gyuyoung.kim ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBjMDE4MDUwLi42YzkzYWYzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjkg QEAKKzIwMTEtMDItMDYgIEd5dXlvdW5nIEtpbSAgPGd5dXlvdW5nLmtpbUBzYW1zdW5nLmNvbT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbRUZMXSBB ZGQgZHVtbXkgZnVuY3Rpb25zIGZvciBIVE1MNSBWaWRlbydzIGNvbnRyb2wgVUkKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTUzOTA2CisKKyAgICAgICAg SW4gV2ViS2l0IEVGTCwgdGhlcmUgYXJlIG5vIGltcGxlbWVudGF0aW9ucyBmb3IgSFRNTDUgY29u dHJvbCBVSS4KKyAgICAgICAgRmlyc3QsIGR1bW15IGZ1bmN0aW9ucyBhcmUgYWRkZWQuCisKKyAg ICAgICAgKiBwbGF0Zm9ybS9lZmwvUmVuZGVyVGhlbWVFZmwuY3BwOgorICAgICAgICAoV2ViQ29y ZTo6UmVuZGVyVGhlbWVFZmw6OmV4dHJhTWVkaWFDb250cm9sc1N0eWxlU2hlZXQpOgorICAgICAg ICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVFZmw6OmZvcm1hdE1lZGlhQ29udHJvbHNDdXJyZW50VGlt ZSk6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUVmbDo6cGFpbnRNZWRpYUZ1bGxzY3Jl ZW5CdXR0b24pOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVFZmw6OnBhaW50TWVkaWFN dXRlQnV0dG9uKToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlh UGxheUJ1dHRvbik6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUVmbDo6cGFpbnRNZWRp YVNlZWtCYWNrQnV0dG9uKToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlclRoZW1lRWZsOjpwYWlu dE1lZGlhU2Vla0ZvcndhcmRCdXR0b24pOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVF Zmw6OnBhaW50TWVkaWFTbGlkZXJUcmFjayk6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVt ZUVmbDo6cGFpbnRNZWRpYVNsaWRlclRodW1iKToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlclRo ZW1lRWZsOjpwYWludE1lZGlhVm9sdW1lU2xpZGVyQ29udGFpbmVyKToKKyAgICAgICAgKFdlYkNv cmU6OlJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlhVm9sdW1lU2xpZGVyVHJhY2spOgorICAgICAg ICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVFZmw6OnBhaW50TWVkaWFWb2x1bWVTbGlkZXJUaHVtYik6 CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUVmbDo6cGFpbnRNZWRpYUN1cnJlbnRUaW1l KToKKyAgICAgICAgKiBwbGF0Zm9ybS9lZmwvUmVuZGVyVGhlbWVFZmwuaDoKKwogMjAxMS0wMi0w NiAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBi eSBEYXJpbiBBZGxlci4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2VmbC9S ZW5kZXJUaGVtZUVmbC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9lZmwvUmVuZGVyVGhl bWVFZmwuY3BwCmluZGV4IDlkZjEyZTkuLjkxMDJkMTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2VmbC9SZW5kZXJUaGVtZUVmbC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vZWZsL1JlbmRlclRoZW1lRWZsLmNwcApAQCAtMTA0Myw0ICsxMDQzLDgzIEBAIGJv b2wgUmVuZGVyVGhlbWVFZmw6OnBhaW50UHJvZ3Jlc3NCYXIoUmVuZGVyT2JqZWN0KiBvLCBjb25z dCBQYWludEluZm8mIGksIGNvbnN0CiB9CiAjZW5kaWYKIAorI2lmIEVOQUJMRShWSURFTykKK1N0 cmluZyBSZW5kZXJUaGVtZUVmbDo6ZXh0cmFNZWRpYUNvbnRyb2xzU3R5bGVTaGVldCgpCit7Cisg ICAgbm90SW1wbGVtZW50ZWQoKTsKKyAgICByZXR1cm4gU3RyaW5nKCk7Cit9CisKK1N0cmluZyBS ZW5kZXJUaGVtZUVmbDo6Zm9ybWF0TWVkaWFDb250cm9sc0N1cnJlbnRUaW1lKGZsb2F0IGN1cnJl bnRUaW1lLCBmbG9hdCBkdXJhdGlvbikgY29uc3QKK3sKKyAgICBub3RJbXBsZW1lbnRlZCgpOwor ICAgIHJldHVybiBTdHJpbmcoKTsKK30KKworYm9vbCBSZW5kZXJUaGVtZUVmbDo6cGFpbnRNZWRp YUZ1bGxzY3JlZW5CdXR0b24oUmVuZGVyT2JqZWN0KiByZW5kZXJPYmplY3QsIGNvbnN0IFBhaW50 SW5mbyYgcGFpbnRJbmZvLCBjb25zdCBJbnRSZWN0JiByZWN0KQoreworICAgIG5vdEltcGxlbWVu dGVkKCk7CisgICAgcmV0dXJuIGZhbHNlOworfQorCitib29sIFJlbmRlclRoZW1lRWZsOjpwYWlu dE1lZGlhTXV0ZUJ1dHRvbihSZW5kZXJPYmplY3QqIHJlbmRlck9iamVjdCwgY29uc3QgUGFpbnRJ bmZvJiBwYWludEluZm8sIGNvbnN0IEludFJlY3QmIHJlY3QpCit7CisgICAgbm90SW1wbGVtZW50 ZWQoKTsKKyAgICByZXR1cm4gZmFsc2U7Cit9CisKK2Jvb2wgUmVuZGVyVGhlbWVFZmw6OnBhaW50 TWVkaWFQbGF5QnV0dG9uKFJlbmRlck9iamVjdCogcmVuZGVyT2JqZWN0LCBjb25zdCBQYWludElu Zm8mIHBhaW50SW5mbywgY29uc3QgSW50UmVjdCYgcmVjdCkKK3sKKyAgICBub3RJbXBsZW1lbnRl ZCgpOworICAgIHJldHVybiBmYWxzZTsKK30KKworYm9vbCBSZW5kZXJUaGVtZUVmbDo6cGFpbnRN ZWRpYVNlZWtCYWNrQnV0dG9uKFJlbmRlck9iamVjdCogcmVuZGVyT2JqZWN0LCBjb25zdCBQYWlu dEluZm8mIHBhaW50SW5mbywgY29uc3QgSW50UmVjdCYgcmVjdCkKK3sKKyAgICBub3RJbXBsZW1l bnRlZCgpOworICAgIHJldHVybiBmYWxzZTsKK30KKworYm9vbCBSZW5kZXJUaGVtZUVmbDo6cGFp bnRNZWRpYVNlZWtGb3J3YXJkQnV0dG9uKFJlbmRlck9iamVjdCogcmVuZGVyT2JqZWN0LCBjb25z dCBQYWludEluZm8mIHBhaW50SW5mbywgY29uc3QgSW50UmVjdCYgcmVjdCkKK3sKKyAgICBub3RJ bXBsZW1lbnRlZCgpOworICAgIHJldHVybiBmYWxzZTsKK30KKworYm9vbCBSZW5kZXJUaGVtZUVm bDo6cGFpbnRNZWRpYVNsaWRlclRyYWNrKFJlbmRlck9iamVjdCogbywgY29uc3QgUGFpbnRJbmZv JiBwYWludEluZm8sIGNvbnN0IEludFJlY3QmIHIpCit7CisgICAgbm90SW1wbGVtZW50ZWQoKTsK KyAgICByZXR1cm4gZmFsc2U7Cit9CisKK2Jvb2wgUmVuZGVyVGhlbWVFZmw6OnBhaW50TWVkaWFT bGlkZXJUaHVtYihSZW5kZXJPYmplY3QqIG8sIGNvbnN0IFBhaW50SW5mbyYgcGFpbnRJbmZvLCBj b25zdCBJbnRSZWN0JiByKQoreworICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAgcmV0dXJuIGZh bHNlOworfQorCitib29sIFJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlhVm9sdW1lU2xpZGVyQ29u dGFpbmVyKFJlbmRlck9iamVjdCosIGNvbnN0IFBhaW50SW5mbyYgcGFpbnRJbmZvLCBjb25zdCBJ bnRSZWN0JiByZWN0KQoreworICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAgcmV0dXJuIGZhbHNl OworfQorCitib29sIFJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlhVm9sdW1lU2xpZGVyVHJhY2so UmVuZGVyT2JqZWN0KiByZW5kZXJPYmplY3QsIGNvbnN0IFBhaW50SW5mbyYgcGFpbnRJbmZvLCBj b25zdCBJbnRSZWN0JiByZWN0KQoreworICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAgcmV0dXJu IGZhbHNlOworfQorCitib29sIFJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlhVm9sdW1lU2xpZGVy VGh1bWIoUmVuZGVyT2JqZWN0KiByZW5kZXJPYmplY3QsIGNvbnN0IFBhaW50SW5mbyYgcGFpbnRJ bmZvLCBjb25zdCBJbnRSZWN0JiByZWN0KQoreworICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAg cmV0dXJuIGZhbHNlOworfQorCitib29sIFJlbmRlclRoZW1lRWZsOjpwYWludE1lZGlhQ3VycmVu dFRpbWUoUmVuZGVyT2JqZWN0KiByZW5kZXJPYmplY3QsIGNvbnN0IFBhaW50SW5mbyYgcGFpbnRJ bmZvLCBjb25zdCBJbnRSZWN0JiByZWN0KQoreworICAgIG5vdEltcGxlbWVudGVkKCk7CisgICAg cmV0dXJuIGZhbHNlOworfQorI2VuZGlmCiB9CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9lZmwvUmVuZGVyVGhlbWVFZmwuaCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Vm bC9SZW5kZXJUaGVtZUVmbC5oCmluZGV4IDA4N2UyYWEuLmQ0ODg3Y2YgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2VmbC9SZW5kZXJUaGVtZUVmbC5oCisrKyBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2VmbC9SZW5kZXJUaGVtZUVmbC5oCkBAIC0xNTMsNiArMTUzLDIzIEBA IHB1YmxpYzoKICAgICB2aXJ0dWFsIGJvb2wgcGFpbnRQcm9ncmVzc0JhcihSZW5kZXJPYmplY3Qq LCBjb25zdCBQYWludEluZm8mLCBjb25zdCBJbnRSZWN0Jik7CiAjZW5kaWYKIAorI2lmIEVOQUJM RShWSURFTykKKyAgICB2aXJ0dWFsIFN0cmluZyBleHRyYU1lZGlhQ29udHJvbHNTdHlsZVNoZWV0 KCk7CisgICAgdmlydHVhbCBTdHJpbmcgZm9ybWF0TWVkaWFDb250cm9sc0N1cnJlbnRUaW1lKGZs b2F0IGN1cnJlbnRUaW1lLCBmbG9hdCBkdXJhdGlvbikgY29uc3Q7CisKKyAgICB2aXJ0dWFsIGJv b2wgcGFpbnRNZWRpYUZ1bGxzY3JlZW5CdXR0b24oUmVuZGVyT2JqZWN0KiwgY29uc3QgUGFpbnRJ bmZvJiwgY29uc3QgSW50UmVjdCYpOworICAgIHZpcnR1YWwgYm9vbCBwYWludE1lZGlhUGxheUJ1 dHRvbihSZW5kZXJPYmplY3QqLCBjb25zdCBQYWludEluZm8mLCBjb25zdCBJbnRSZWN0Jik7Cisg ICAgdmlydHVhbCBib29sIHBhaW50TWVkaWFNdXRlQnV0dG9uKFJlbmRlck9iamVjdCosIGNvbnN0 IFBhaW50SW5mbyYsIGNvbnN0IEludFJlY3QmKTsKKyAgICB2aXJ0dWFsIGJvb2wgcGFpbnRNZWRp YVNlZWtCYWNrQnV0dG9uKFJlbmRlck9iamVjdCosIGNvbnN0IFBhaW50SW5mbyYsIGNvbnN0IElu dFJlY3QmKTsKKyAgICB2aXJ0dWFsIGJvb2wgcGFpbnRNZWRpYVNlZWtGb3J3YXJkQnV0dG9uKFJl bmRlck9iamVjdCosIGNvbnN0IFBhaW50SW5mbyYsIGNvbnN0IEludFJlY3QmKTsKKyAgICB2aXJ0 dWFsIGJvb2wgcGFpbnRNZWRpYVNsaWRlclRyYWNrKFJlbmRlck9iamVjdCosIGNvbnN0IFBhaW50 SW5mbyYsIGNvbnN0IEludFJlY3QmKTsKKyAgICB2aXJ0dWFsIGJvb2wgcGFpbnRNZWRpYVNsaWRl clRodW1iKFJlbmRlck9iamVjdCosIGNvbnN0IFBhaW50SW5mbyYsIGNvbnN0IEludFJlY3QmKTsK KyAgICB2aXJ0dWFsIGJvb2wgcGFpbnRNZWRpYVZvbHVtZVNsaWRlckNvbnRhaW5lcihSZW5kZXJP YmplY3QqLCBjb25zdCBQYWludEluZm8mLCBjb25zdCBJbnRSZWN0Jik7CisgICAgdmlydHVhbCBi b29sIHBhaW50TWVkaWFWb2x1bWVTbGlkZXJUcmFjayhSZW5kZXJPYmplY3QqLCBjb25zdCBQYWlu dEluZm8mLCBjb25zdCBJbnRSZWN0Jik7CisgICAgdmlydHVhbCBib29sIHBhaW50TWVkaWFWb2x1 bWVTbGlkZXJUaHVtYihSZW5kZXJPYmplY3QqLCBjb25zdCBQYWludEluZm8mLCBjb25zdCBJbnRS ZWN0Jik7CisgICAgdmlydHVhbCBib29sIHBhaW50TWVkaWFDdXJyZW50VGltZShSZW5kZXJPYmpl Y3QqLCBjb25zdCBQYWludEluZm8mLCBjb25zdCBJbnRSZWN0Jik7CisjZW5kaWYKKwogcHJvdGVj dGVkOgogICAgIHN0YXRpYyBmbG9hdCBkZWZhdWx0Rm9udFNpemU7CiAK