Bug 104505 – [BlackBerry] Flash content is being rendered as garbled characters when Flashplayer is disabled

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Max Feil]

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.

[Max Feil]

Created attachment 178468 [details]
Patch

[WebKit Review Bot]

Comment on attachment 178468 [details]
Patch

Attachment 178468 [details] 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

[Max Feil]

Comment on attachment 178468 [details]
Patch

Resetting commit queue "-" from impossible failure. This patch affects BlackBerry port only.

[Antonio Gomes]

Comment on attachment 178468 [details]
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.

[Max Feil]

(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 :)

[Max Feil]

Comment on attachment 178468 [details]
Patch

Requesting commit-queue. The cr-linux test failure is a false alarm.

[WebKit Review Bot]

Comment on attachment 178468 [details]
Patch

Clearing flags on attachment: 178468

Committed r137210: <http://trac.webkit.org/changeset/137210>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.