When I set Flash to "Block" in Safari 9.0, I don't see a "Plug-in blocked" message on homestarrunner.com, but I should. The placeholder is actually on the page, but isn't drawn.
<rdar://problem/22920949>
Created attachment 262322 [details] First patch
Comment on attachment 262322 [details] First patch View in context: https://bugs.webkit.org/attachment.cgi?id=262322&action=review Seems reasonable to me but Anders is the best person for plugin patches; Ccing him. > Source/WebCore/ChangeLog:4 > + No newline here. > Source/WebCore/ChangeLog:6 > + <rdar://problem/22920949> > + https://bugs.webkit.org/show_bug.cgi?id=149741 These are backwards IMO (but nobody strictly follows any rule here anyway). > Source/WebCore/ChangeLog:12 > + No new tests are added. Why not? From a cursory glance, this seems testable.
Also, is this a regression? I am pretty sure the unavailable plugin indicator has worked for blocked Flash in the past.
Also, a pared down version of your first comment would make a better title than the title that you chose (bug title/commit title should describe the problem, not the solution).
Or even the title from the bug would be fine :D
Created attachment 262323 [details] Second attempt, addressing Tim's feedback Handle Tim's feedback. Still thinking about testing.
(In reply to comment #4) > Also, is this a regression? I am pretty sure the unavailable plugin > indicator has worked for blocked Flash in the past. Yes, this is a regression.
Comment on attachment 262323 [details] Second attempt, addressing Tim's feedback View in context: https://bugs.webkit.org/attachment.cgi?id=262323&action=review > Source/WebKit2/ChangeLog:4 > + You left a newline here :D Perhaps you can repurpose LayoutTests/plugins/unavailable-plugin-indicator-obscurity.html to test this somehow? Or something like it?
(In reply to comment #8) > (In reply to comment #4) > > Also, is this a regression? I am pretty sure the unavailable plugin > > indicator has worked for blocked Flash in the past. > > Yes, this is a regression. Do you know when? That would also be good title-fodder, of course :D
(In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #4) > > > Also, is this a regression? I am pretty sure the unavailable plugin > > > indicator has worked for blocked Flash in the past. > > > > Yes, this is a regression. > > Do you know when? That would also be good title-fodder, of course :D I haven't proven it by making my own build, but I strongly suspect that r181562 was the cause.
Created attachment 262324 [details] Third attempt Actually removing the newline that Tim requested be removed. Still looking into testing.
Comment on attachment 262324 [details] Third attempt View in context: https://bugs.webkit.org/attachment.cgi?id=262324&action=review > Source/WebCore/plugins/PluginData.cpp:127 > + for (unsigned i = 0; i < mimes.size(); ++i) { Please use a modern style C++ loop.
(In reply to comment #13) > Comment on attachment 262324 [details] > Third attempt > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262324&action=review > > > Source/WebCore/plugins/PluginData.cpp:127 > > + for (unsigned i = 0; i < mimes.size(); ++i) { > > Please use a modern style C++ loop. This file is full of parallel vectors; they need the index to index into to the other ones!
Comment on attachment 262324 [details] Third attempt Clearing flags on attachment: 262324 Committed r190547: <http://trac.webkit.org/changeset/190547>
All reviewed patches have been landed. Closing bug.