WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
149741
"Plug-in is blocked" message doesn't draw correctly
https://bugs.webkit.org/show_bug.cgi?id=149741
Summary
"Plug-in is blocked" message doesn't draw correctly
Ricky Mondello
Reported
2015-10-02 00:26:29 PDT
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.
Attachments
First patch
(8.56 KB, patch)
2015-10-02 00:53 PDT
,
Ricky Mondello
no flags
Details
Formatted Diff
Diff
Second attempt, addressing Tim's feedback
(8.51 KB, patch)
2015-10-02 01:05 PDT
,
Ricky Mondello
no flags
Details
Formatted Diff
Diff
Third attempt
(8.36 KB, patch)
2015-10-02 01:27 PDT
,
Ricky Mondello
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Ricky Mondello
Comment 1
2015-10-02 00:26:57 PDT
<
rdar://problem/22920949
>
Ricky Mondello
Comment 2
2015-10-02 00:53:14 PDT
Created
attachment 262322
[details]
First patch
Tim Horton
Comment 3
2015-10-02 00:59:38 PDT
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.
Tim Horton
Comment 4
2015-10-02 01:00:40 PDT
Also, is this a regression? I am pretty sure the unavailable plugin indicator has worked for blocked Flash in the past.
Tim Horton
Comment 5
2015-10-02 01:01:18 PDT
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).
Tim Horton
Comment 6
2015-10-02 01:01:36 PDT
Or even the title from the bug would be fine :D
Ricky Mondello
Comment 7
2015-10-02 01:05:50 PDT
Created
attachment 262323
[details]
Second attempt, addressing Tim's feedback Handle Tim's feedback. Still thinking about testing.
Ricky Mondello
Comment 8
2015-10-02 01:06:56 PDT
(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.
Tim Horton
Comment 9
2015-10-02 01:08:29 PDT
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?
Tim Horton
Comment 10
2015-10-02 01:09:41 PDT
(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
Ricky Mondello
Comment 11
2015-10-02 01:21:02 PDT
(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.
Ricky Mondello
Comment 12
2015-10-02 01:27:22 PDT
Created
attachment 262324
[details]
Third attempt Actually removing the newline that Tim requested be removed. Still looking into testing.
Alexey Proskuryakov
Comment 13
2015-10-02 19:45:01 PDT
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.
Tim Horton
Comment 14
2015-10-02 21:01:12 PDT
(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!
WebKit Commit Bot
Comment 15
2015-10-03 19:38:50 PDT
Comment on
attachment 262324
[details]
Third attempt Clearing flags on attachment: 262324 Committed
r190547
: <
http://trac.webkit.org/changeset/190547
>
WebKit Commit Bot
Comment 16
2015-10-03 19:38:56 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug