104505 2012-12-09 21:20:29 -0800 [BlackBerry] Flash content is being rendered as garbled characters when Flashplayer is disabled 2012-12-10 14:58:45 -0800 1 1 1 Unclassified WebKit WebKit BlackBerry 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mfeil mfeil dglazkov logingx mifenton rwlbuis tonikitoo webkit.review.bot yong.li.webkit oldest_to_newest 786964 0 mfeil 2012-12-09 21:20:29 -0800 The fix for bug 101274 did (will?) cause a regression because it made the Flashplayer plugin get disabled without disabling all plugins. This was causing the swf contents to be rendered as garbled text characters where it should just be blank. See platform PR258003. No layout test is included here for the same reason as in bug 101274. This patch goes along with that work. 786971 1 178468 mfeil 2012-12-09 21:30:18 -0800 Created attachment 178468 Patch 786982 2 178468 webkit.review.bot 2012-12-09 22:12:12 -0800 Comment on attachment 178468 Patch Attachment 178468 did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15239293 New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html 786995 3 178468 mfeil 2012-12-09 22:28:42 -0800 Comment on attachment 178468 Patch Resetting commit queue "-" from impossible failure. This patch affects BlackBerry port only. 787213 4 178468 tonikitoo 2012-12-10 05:10:26 -0800 Comment on attachment 178468 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178468&action=review > Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:-348 > - if (m_frame->loader() && m_frame->loader()->subframeLoader() && !url.isNull()) As a side note, I am not sure loader() needs to be null checked. I might be wrong, but it might worth it to confirm. > Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:354 > + if (mimeType != "application/x-shockwave-flash" && m_frame->loader() && m_frame->loader()->subframeLoader() && !url.isNull()) Looks good. I would do a static helper function maybe to get a shorter line. 787558 5 mfeil 2012-12-10 11:11:23 -0800 (In reply to comment #4) > As a side note, I am not sure loader() needs to be null checked. I might be wrong, but it might worth it to confirm. I prefer not to change other developers' reviewed code if it's unrelated to my patch and is of low possible gain. If you feel strongly about this case, you can file a bug to check WebKit code for this and all other cases where loader() might be null checked. > Looks good. I would do a static helper function maybe to get a shorter line. Thanks! Looks ok in my editor :) 787648 6 178468 mfeil 2012-12-10 12:54:32 -0800 Comment on attachment 178468 Patch Requesting commit-queue. The cr-linux test failure is a false alarm. 787739 7 178468 webkit.review.bot 2012-12-10 14:58:41 -0800 Comment on attachment 178468 Patch Clearing flags on attachment: 178468 Committed r137210: <http://trac.webkit.org/changeset/137210> 787740 8 webkit.review.bot 2012-12-10 14:58:45 -0800 All reviewed patches have been landed. Closing bug. 178468 2012-12-09 21:30:18 -0800 2012-12-10 14:58:41 -0800 Patch bug-104505-20121210002740.patch text/plain 2786 mfeil U3VidmVyc2lvbiBSZXZpc2lvbjogMTM3MTA1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2Js YWNrYmVycnkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9ibGFja2JlcnJ5L0NoYW5nZUxvZwpp bmRleCAyNDQxZjllOWRjMjBkNmJlNDk5ZjdkYzcwOTY4ZjMwMzcxNTVlNDZmLi5kOGFjYzhjODVj NjEyY2UyMGM0OTlhNGY0YjQwYjE3YTllNjA1ZjY2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 L2JsYWNrYmVycnkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvYmxhY2tiZXJyeS9DaGFu Z2VMb2cKQEAgLTEsMyArMSwxOSBAQAorMjAxMi0xMi0wOSAgTWF4IEZlaWwgIDxtZmVpbEByaW0u Y29tPgorCisgICAgICAgIFtCbGFja0JlcnJ5XSBGbGFzaCBjb250ZW50IGlzIGJlaW5nIHJlbmRl cmVkIGFzIGdhcmJsZWQgY2hhcmFjdGVycyB3aGVuIEZsYXNocGxheWVyIGlzIGRpc2FibGVkCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDQ1MDUKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgY2hlY2sg dG8gYWxsb3cgdmlkZW8gYW5kIGF1ZGlvIGFzIHRoZSBzb3VyY2Ugb2YgZW1iZWQgYW5kCisgICAg ICAgIG9iamVjdCBlbGVtZW50cyBpcyBjYXVzaW5nIGEgcHJvYmxlbSB3aGVuIHRoZSBGbGFzaHBs YXllciBwbHVnaW4KKyAgICAgICAgaXMgZGlzYWJsZWQuIFdlIGhhdmUgdG8gdGFrZSBGbGFzaCBj b250ZW50IGludG8gYWNjb3VudCBpbiB0aGlzCisgICAgICAgIGNoZWNrIG90aGVyd2lzZSB0aGUg c3dmIGNvbnRlbnRzIHdpbGwgYmUgcmVuZGVyZWQgYXMgZ2FyYmxlZAorICAgICAgICB0ZXh0IGNo YXJhY3RlcnMgd2hlcmUgdGhlcmUgc2hvdWxkIGp1c3QgYmUgYmxhbmsgc3BhY2UuCisKKyAgICAg ICAgKiBXZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudEJsYWNrQmVycnkuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6RnJhbWVMb2FkZXJDbGllbnRCbGFja0JlcnJ5OjpjcmVhdGVQbHVnaW4p OgorCiAyMDEyLTEyLTA4ICBKYWNreSBKaWFuZyAgPHpoYWppYW5nQHJpbS5jb20+CiAKICAgICAg ICAgW0JsYWNrQmVycnldIEdvb2dsZSByZXN1bHRzIHBhZ2UgcmVuZGVyaW5nIGlzc3VlIHdpdGgg UlRMIGxhbmd1YWdlcyBsaWtlIGFyYWJpYy9oZWJyZXcKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQvYmxhY2tiZXJyeS9XZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudEJsYWNrQmVycnku Y3BwIGIvU291cmNlL1dlYktpdC9ibGFja2JlcnJ5L1dlYkNvcmVTdXBwb3J0L0ZyYW1lTG9hZGVy Q2xpZW50QmxhY2tCZXJyeS5jcHAKaW5kZXggNzNhNjZlNTdjNzU1MjRjZTc4ODhmMTg0MzYyNTY0 NjM0MWM0ZjYyMy4uMTMyNmJmYTZiZTdlYjFlYjlmYTYxNDc2NGRkMDY0Y2Y0YjUxODM4YiAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYktpdC9ibGFja2JlcnJ5L1dlYkNvcmVTdXBwb3J0L0ZyYW1lTG9h ZGVyQ2xpZW50QmxhY2tCZXJyeS5jcHAKKysrIGIvU291cmNlL1dlYktpdC9ibGFja2JlcnJ5L1dl YkNvcmVTdXBwb3J0L0ZyYW1lTG9hZGVyQ2xpZW50QmxhY2tCZXJyeS5jcHAKQEAgLTM0NCw4ICsz NDQsMTQgQEAgUGFzc1JlZlB0cjxXaWRnZXQ+IEZyYW1lTG9hZGVyQ2xpZW50QmxhY2tCZXJyeTo6 Y3JlYXRlUGx1Z2luKGNvbnN0IEludFNpemUmIHBsdWcKICAgICBpZiAoUGx1Z2luRGF0YWJhc2U6 Omluc3RhbGxlZFBsdWdpbnMoKS0+aXNNSU1FVHlwZVJlZ2lzdGVyZWQobWltZVR5cGUpKQogICAg ICAgICByZXR1cm4gUGx1Z2luVmlldzo6Y3JlYXRlKG1fZnJhbWUsIHBsdWdpblNpemUsIGVsZW1l bnQsIHVybCwgcGFyYW1OYW1lcywgcGFyYW1WYWx1ZXMsIG1pbWVUeXBlLCBsb2FkTWFudWFsbHkp OwogCi0gICAgLy8gSWYgaXQncyBub3QgdGhlIHBsdWdpbiB0eXBlIHdlIHN1cHBvcnQsIHRyeSBs b2FkIGRpcmVjdGx5IGZyb20gYnJvd3Nlci4KLSAgICBpZiAobV9mcmFtZS0+bG9hZGVyKCkgJiYg bV9mcmFtZS0+bG9hZGVyKCktPnN1YmZyYW1lTG9hZGVyKCkgJiYgIXVybC5pc051bGwoKSkKKyAg ICAvLyBUaGlzIGlzIG5vdCBhIHBsdWdpbiB0eXBlIHRoYXQgaXMgY3VycmVudGx5IHN1cHBvcnRl ZCBvciBlbmFibGVkLiBUcnkKKyAgICAvLyB0byBsb2FkIHRoZSB1cmwgZGlyZWN0bHkuIFRoaXMg Y2hlY2sgaXMgcGVyZm9ybWVkIHRvIGFsbG93IHZpZGVvIGFuZAorICAgIC8vIGF1ZGlvIHRvIGJl IHJlZmVyZW5jZWQgYXMgdGhlIHNvdXJjZSBvZiBlbWJlZCBvciBvYmplY3QgZWxlbWVudHMuCisg ICAgLy8gRm9yIG1lZGlhIG9mIHRoaXMga2luZCB0aGUgbWltZSB0eXBlIHBhc3NlZCBpbnRvIHRo aXMgZnVuY3Rpb24KKyAgICAvLyB3aWxsIGdlbmVyYWxseSBiZSBhIHZhbGlkIG1lZGlhIG1pbWUg dHlwZSwgb3IgaXQgbWF5IGJlIG51bGwuIFdlCisgICAgLy8gZXhwbGljaXRseSBjaGVjayBmb3Ig Rmxhc2ggY29udGVudCBzbyBpdCBkb2VzIG5vdCBnZXQgcmVuZGVyZWQgYXMKKyAgICAvLyB0ZXh0 IGF0IHRoaXMgcG9pbnQsIHByb2R1Y2luZyBnYXJibGVkIGNoYXJhY3RlcnMuCisgICAgaWYgKG1p bWVUeXBlICE9ICJhcHBsaWNhdGlvbi94LXNob2Nrd2F2ZS1mbGFzaCIgJiYgbV9mcmFtZS0+bG9h ZGVyKCkgJiYgbV9mcmFtZS0+bG9hZGVyKCktPnN1YmZyYW1lTG9hZGVyKCkgJiYgIXVybC5pc051 bGwoKSkKICAgICAgICAgbV9mcmFtZS0+bG9hZGVyKCktPnN1YmZyYW1lTG9hZGVyKCktPnJlcXVl c3RGcmFtZShlbGVtZW50LCB1cmwsIFN0cmluZygpKTsKIAogICAgIHJldHVybiAwOwo=