64683 2011-07-18 02:16:33 -0700 [EFL] Method ewk_context_menu_destroy returns EINA_FALSE if CONTEXT_MENUS is disabled 2011-07-25 00:09:55 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) All Linux RESOLVED FIXED P2 Normal --- 0 g.czajkowski webkit-unassigned gyuyoung.kim gyuyoung.kim lucas.de.marchi rakuco webkit.review.bot oldest_to_newest 438382 0 g.czajkowski 2011-07-18 02:16:33 -0700 If CONTEXT_MENUS macro is disabled ewk_context_menu_destroy does nothing so it returns EINA_FALSE and omits NULL checking. 438383 1 101139 g.czajkowski 2011-07-18 02:17:46 -0700 Created attachment 101139 proposed patch 438424 2 rakuco 2011-07-18 05:57:17 -0700 The ChangeLog entry sounds a bit confusing to me -- the one-line description looks like the method _currently_ does what is being described, not that this is what is being changed. I see that you are submitting a few separate patches related to context menus. Do you plan on adding these #if checks to the other functions in ewk_contextmenu.cpp? 438482 3 g.czajkowski 2011-07-18 07:46:38 -0700 (In reply to comment #2) > The ChangeLog entry sounds a bit confusing to me -- the one-line description looks like the method _currently_ does what is being described, not that this is what is being changed. > Ok, I will fix ChangeLog. > I see that you are submitting a few separate patches related to context menus. Do you plan on adding these #if checks to the other functions in ewk_contextmenu.cpp? I've sent two patches related to the context menu. First fixes build break, second one changes returned value. There aren't more references to WebCore's context menu object (controller) by API that are not surrounded by CONTEXT_MENUS macro. I don't think so others methods should check this macro. What is your opinion on that? 438495 4 rakuco 2011-07-18 08:35:51 -0700 (In reply to comment #3) > > I see that you are submitting a few separate patches related to context menus. Do you plan on adding these #if checks to the other functions in ewk_contextmenu.cpp? > > I've sent two patches related to the context menu. First fixes build break, second one changes returned value. > There aren't more references to WebCore's context menu object (controller) by API that are not surrounded by CONTEXT_MENUS macro. I don't think so others methods should check this macro. What is your opinion on that? Let's not fix what's not broken ;) If the rest works, I think it's OK to leave it as it is. 438991 5 101277 g.czajkowski 2011-07-18 23:49:19 -0700 Created attachment 101277 ChangeLog fix 438993 6 webkit.review.bot 2011-07-18 23:52:07 -0700 Attachment 101277 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1 Source/WebKit/efl/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 438996 7 101278 g.czajkowski 2011-07-18 23:55:17 -0700 Created attachment 101278 ChangeLog fix Incorrect URL in the previous patch. 439005 8 gyuyoung.kim 2011-07-19 00:18:20 -0700 LGTM. 439108 9 rakuco 2011-07-19 06:02:17 -0700 r+ from my side too. 441569 10 101278 webkit.review.bot 2011-07-25 00:09:50 -0700 Comment on attachment 101278 ChangeLog fix Clearing flags on attachment: 101278 Committed r91658: <http://trac.webkit.org/changeset/91658> 441570 11 webkit.review.bot 2011-07-25 00:09:55 -0700 All reviewed patches have been landed. Closing bug. 101139 2011-07-18 02:17:46 -0700 2011-07-18 23:49:19 -0700 proposed patch menu_destroy_fix.patch text/plain 1486 g.czajkowski ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv ZWZsL0NoYW5nZUxvZwppbmRleCA4M2E1N2Q1Li43YmIxNWI0IDEwMDc1NQotLS0gYS9Tb3VyY2Uv V2ViS2l0L2VmbC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9lZmwvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTEtMDctMTggIEdyemVnb3J6IEN6YWprb3dza2kgIDxnLmN6YWpr b3dza2lAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0VGTF0gTWV0aG9kIGV3a19jb250ZXh0X21l bnVfZGVzdHJveSByZXR1cm5zIEVJTkFfRkFMU0UgaWYgQ09OVEVYVF9NRU5VUyBpcyBkaXNhYmxl ZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjQ2ODMK KworICAgICAgICBJZiBDT05URVhUX01FTlVTIG1hY3JvIGlzIGRpc2FibGVkIGV3a19jb250ZXh0 X21lbnVfZGVzdHJveSBkb2VzIG5vdGhpbmcgc28gaXQKKyAgICAgICAgcmV0dXJucyBFSU5BX0ZB TFNFIGFuZCBvbWl0cyBOVUxMIGNoZWNraW5nLgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgICogZXdrL2V3a19jb250ZXh0bWVudS5jcHA6CisgICAgICAg IChld2tfY29udGV4dF9tZW51X2Rlc3Ryb3kpOgorCiAyMDExLTA3LTE1ICBEYW4gQmVybnN0ZWlu ICA8bWl0ekBhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTjogTW91c2UgY3Vyc29yIGRv ZXNu4oCZdCBoaWRlIHdoZW4gZnVsbCBzY3JlZW4gdmlkZW8gSFVEIGhpZGVzCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViS2l0L2VmbC9ld2svZXdrX2NvbnRleHRtZW51LmNwcCBiL1NvdXJjZS9XZWJL aXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwCmluZGV4IDEyODIwOGQuLmQ3YTAxNGEgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwCisrKyBi L1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwCkBAIC0xMDIsMTQgKzEw MiwxNCBAQCB2b2lkIGV3a19jb250ZXh0X21lbnVfdW5yZWYoRXdrX0NvbnRleHRfTWVudSogbWVu dSkKICAqLwogRWluYV9Cb29sIGV3a19jb250ZXh0X21lbnVfZGVzdHJveShFd2tfQ29udGV4dF9N ZW51KiBtZW51KQogewotICAgIEVJTkFfU0FGRVRZX09OX05VTExfUkVUVVJOX1ZBTChtZW51LCBF SU5BX0ZBTFNFKTsKICNpZiBFTkFCTEUoQ09OVEVYVF9NRU5VUykKKyAgICBFSU5BX1NBRkVUWV9P Tl9OVUxMX1JFVFVSTl9WQUwobWVudSwgRUlOQV9GQUxTRSk7CiAgICAgRUlOQV9TQUZFVFlfT05f TlVMTF9SRVRVUk5fVkFMKG1lbnUtPmNvbnRyb2xsZXIsIEVJTkFfRkFMU0UpOwotCiAgICAgbWVu dS0+Y29udHJvbGxlci0+Y2xlYXJDb250ZXh0TWVudSgpOwotI2VuZGlmCi0KICAgICByZXR1cm4g RUlOQV9UUlVFOworI2Vsc2UgCisgICAgcmV0dXJuIEVJTkFfRkFMU0U7CisjZW5kaWYKIH0KIAog LyoqCg== 101277 2011-07-18 23:49:19 -0700 2011-07-18 23:55:17 -0700 ChangeLog fix menu_destroy_fix.patch text/plain 1477 g.czajkowski ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv ZWZsL0NoYW5nZUxvZwppbmRleCA4M2E1N2Q1Li4yZDU1MjY0IDEwMDc1NQotLS0gYS9Tb3VyY2Uv V2ViS2l0L2VmbC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9lZmwvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTEtMDctMTggIEdyemVnb3J6IEN6YWprb3dza2kgIDxnLmN6YWpr b3dza2lAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0VGTF0gTWV0aG9kIGV3a19jb250ZXh0X21l bnVfZGVzdHJveSByZXR1cm5zIEVJTkFfRkFMU0UgaWYgQ09OVEVYVF9NRU5VUyBtYWNybyBpcyBk aXNhYmxlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 WFhYWFhYCisKKyAgICAgICAgRml4IHJldHVybmVkIHZhbHVlIGFuZCBvbWl0IE5VTEwgY2hlY2tp bmcgaWYgQ09OVEVYVF9NRU5VUyBtYWNybyBpcyBkaXNhYmxlZAorICAgICAgICBpbiBld2tfY29u dGV4dF9tZW51X2Rlc3Ryb3kuIAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgICogZXdrL2V3a19jb250ZXh0bWVudS5jcHA6CisgICAgICAgIChld2tfY29u dGV4dF9tZW51X2Rlc3Ryb3kpOgorCiAyMDExLTA3LTE1ICBEYW4gQmVybnN0ZWluICA8bWl0ekBh cHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTjogTW91c2UgY3Vyc29yIGRvZXNu4oCZdCBo aWRlIHdoZW4gZnVsbCBzY3JlZW4gdmlkZW8gSFVEIGhpZGVzCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViS2l0L2VmbC9ld2svZXdrX2NvbnRleHRtZW51LmNwcCBiL1NvdXJjZS9XZWJLaXQvZWZsL2V3 ay9ld2tfY29udGV4dG1lbnUuY3BwCmluZGV4IDEyODIwOGQuLmQ3YTAxNGEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJLaXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwCkBAIC0xMDIsMTQgKzEwMiwxNCBAQCB2 b2lkIGV3a19jb250ZXh0X21lbnVfdW5yZWYoRXdrX0NvbnRleHRfTWVudSogbWVudSkKICAqLwog RWluYV9Cb29sIGV3a19jb250ZXh0X21lbnVfZGVzdHJveShFd2tfQ29udGV4dF9NZW51KiBtZW51 KQogewotICAgIEVJTkFfU0FGRVRZX09OX05VTExfUkVUVVJOX1ZBTChtZW51LCBFSU5BX0ZBTFNF KTsKICNpZiBFTkFCTEUoQ09OVEVYVF9NRU5VUykKKyAgICBFSU5BX1NBRkVUWV9PTl9OVUxMX1JF VFVSTl9WQUwobWVudSwgRUlOQV9GQUxTRSk7CiAgICAgRUlOQV9TQUZFVFlfT05fTlVMTF9SRVRV Uk5fVkFMKG1lbnUtPmNvbnRyb2xsZXIsIEVJTkFfRkFMU0UpOwotCiAgICAgbWVudS0+Y29udHJv bGxlci0+Y2xlYXJDb250ZXh0TWVudSgpOwotI2VuZGlmCi0KICAgICByZXR1cm4gRUlOQV9UUlVF OworI2Vsc2UgCisgICAgcmV0dXJuIEVJTkFfRkFMU0U7CisjZW5kaWYKIH0KIAogLyoqCg== 101278 2011-07-18 23:55:17 -0700 2011-07-25 00:09:50 -0700 ChangeLog fix menu_destroy_fix.patch text/plain 1476 g.czajkowski ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZWZsL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv ZWZsL0NoYW5nZUxvZwppbmRleCA4M2E1N2Q1Li4yZDU1MjY0IDEwMDc1NQotLS0gYS9Tb3VyY2Uv V2ViS2l0L2VmbC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9lZmwvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTEtMDctMTggIEdyemVnb3J6IEN6YWprb3dza2kgIDxnLmN6YWpr b3dza2lAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0VGTF0gTWV0aG9kIGV3a19jb250ZXh0X21l bnVfZGVzdHJveSByZXR1cm5zIEVJTkFfRkFMU0UgaWYgQ09OVEVYVF9NRU5VUyBtYWNybyBpcyBk aXNhYmxlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NjQ2ODMKKworICAgICAgICBGaXggcmV0dXJuZWQgdmFsdWUgYW5kIG9taXQgTlVMTCBjaGVja2lu ZyBpZiBDT05URVhUX01FTlVTIG1hY3JvIGlzIGRpc2FibGVkCisgICAgICAgIGluIGV3a19jb250 ZXh0X21lbnVfZGVzdHJveS4gCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgKiBld2svZXdrX2NvbnRleHRtZW51LmNwcDoKKyAgICAgICAgKGV3a19jb250 ZXh0X21lbnVfZGVzdHJveSk6CisKIDIwMTEtMDctMTUgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFw cGxlLmNvbT4KIAogICAgICAgICBSRUdSRVNTSU9OOiBNb3VzZSBjdXJzb3IgZG9lc27igJl0IGhp ZGUgd2hlbiBmdWxsIHNjcmVlbiB2aWRlbyBIVUQgaGlkZXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQvZWZsL2V3ay9ld2tfY29udGV4dG1lbnUuY3BwIGIvU291cmNlL1dlYktpdC9lZmwvZXdr L2V3a19jb250ZXh0bWVudS5jcHAKaW5kZXggMTI4MjA4ZC4uZDdhMDE0YSAxMDA2NDQKLS0tIGEv U291cmNlL1dlYktpdC9lZmwvZXdrL2V3a19jb250ZXh0bWVudS5jcHAKKysrIGIvU291cmNlL1dl YktpdC9lZmwvZXdrL2V3a19jb250ZXh0bWVudS5jcHAKQEAgLTEwMiwxNCArMTAyLDE0IEBAIHZv aWQgZXdrX2NvbnRleHRfbWVudV91bnJlZihFd2tfQ29udGV4dF9NZW51KiBtZW51KQogICovCiBF aW5hX0Jvb2wgZXdrX2NvbnRleHRfbWVudV9kZXN0cm95KEV3a19Db250ZXh0X01lbnUqIG1lbnUp CiB7Ci0gICAgRUlOQV9TQUZFVFlfT05fTlVMTF9SRVRVUk5fVkFMKG1lbnUsIEVJTkFfRkFMU0Up OwogI2lmIEVOQUJMRShDT05URVhUX01FTlVTKQorICAgIEVJTkFfU0FGRVRZX09OX05VTExfUkVU VVJOX1ZBTChtZW51LCBFSU5BX0ZBTFNFKTsKICAgICBFSU5BX1NBRkVUWV9PTl9OVUxMX1JFVFVS Tl9WQUwobWVudS0+Y29udHJvbGxlciwgRUlOQV9GQUxTRSk7Ci0KICAgICBtZW51LT5jb250cm9s bGVyLT5jbGVhckNvbnRleHRNZW51KCk7Ci0jZW5kaWYKLQogICAgIHJldHVybiBFSU5BX1RSVUU7 CisjZWxzZSAKKyAgICByZXR1cm4gRUlOQV9GQUxTRTsKKyNlbmRpZgogfQogCiAvKioK