WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
81907
[chromium] Plug-in failing to load shouldn't say "Missing Plug-in"
https://bugs.webkit.org/show_bug.cgi?id=81907
Summary
[chromium] Plug-in failing to load shouldn't say "Missing Plug-in"
Bernhard Bauer
Reported
2012-03-22 07:24:58 PDT
[chromium] Plug-in failing to load shouldn't say "Missing Plug-in"
Attachments
Patch
(2.00 KB, patch)
2012-03-22 07:29 PDT
,
Bernhard Bauer
no flags
Details
Formatted Diff
Diff
Patch
(4.17 KB, patch)
2012-03-22 10:32 PDT
,
Bernhard Bauer
no flags
Details
Formatted Diff
Diff
Patch
(1.35 KB, patch)
2012-03-26 05:12 PDT
,
Bernhard Bauer
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-03
(9.71 MB, application/zip)
2012-03-26 05:45 PDT
,
WebKit Review Bot
no flags
Details
Patch
(1.36 KB, patch)
2012-03-27 01:13 PDT
,
Bernhard Bauer
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Bernhard Bauer
Comment 1
2012-03-22 07:29:56 PDT
Created
attachment 133255
[details]
Patch
WebKit Review Bot
Comment 2
2012-03-22 07:32:36 PDT
Please wait for approval from
abarth@webkit.org
,
dglazkov@chromium.org
,
fishd@chromium.org
,
jamesr@chromium.org
or
tkent@chromium.org
before submitting, as this patch contains changes to the Chromium public API. See also
https://trac.webkit.org/wiki/ChromiumWebKitAPI
.
Bernhard Bauer
Comment 3
2012-03-22 07:34:29 PDT
See also
http://crbug.com/119462
for the Chromium bug.
Adam Barth
Comment 4
2012-03-22 10:16:20 PDT
Comment on
attachment 133255
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=133255&action=review
> Source/WebKit/chromium/public/platform/WebLocalizedString.h:78 > + MissingPluginText,
Should we sort this list?
> Source/WebKit/chromium/src/LocalizedStrings.cpp:213 > + String text = query(WebLocalizedString::MissingPluginText); > + if (!text.isEmpty()) > + return text; > notImplemented(); > return String("Missing Plug-in");
Do you plan to remove this fallback once Chromium is updated? Should we remove the notImplemented() call now that it's implemented?
Bernhard Bauer
Comment 5
2012-03-22 10:32:02 PDT
Created
attachment 133292
[details]
Patch
Bernhard Bauer
Comment 6
2012-03-22 10:33:46 PDT
(In reply to
comment #4
)
> (From update of
attachment 133255
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=133255&action=review
> > > Source/WebKit/chromium/public/platform/WebLocalizedString.h:78 > > + MissingPluginText, > > Should we sort this list?
Good idea. Done.
> > Source/WebKit/chromium/src/LocalizedStrings.cpp:213 > > + String text = query(WebLocalizedString::MissingPluginText); > > + if (!text.isEmpty()) > > + return text; > > notImplemented(); > > return String("Missing Plug-in"); > > Do you plan to remove this fallback once Chromium is updated? Should we remove the notImplemented() call now that it's implemented?
Yes, my plan is to remove the fallback and the notImplemented() when we have the strings in Chromium.
WebKit Review Bot
Comment 7
2012-03-22 11:23:30 PDT
Comment on
attachment 133292
[details]
Patch Clearing flags on attachment: 133292 Committed
r111734
: <
http://trac.webkit.org/changeset/111734
>
WebKit Review Bot
Comment 8
2012-03-22 11:23:35 PDT
All reviewed patches have been landed. Closing bug.
Bernhard Bauer
Comment 9
2012-03-26 05:09:02 PDT
I still need to clean up the fallback code.
Bernhard Bauer
Comment 10
2012-03-26 05:12:36 PDT
Created
attachment 133779
[details]
Patch
WebKit Review Bot
Comment 11
2012-03-26 05:45:24 PDT
Comment on
attachment 133779
[details]
Patch
Attachment 133779
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/12129822
New failing tests: plugins/embed-attributes-style.html
WebKit Review Bot
Comment 12
2012-03-26 05:45:32 PDT
Created
attachment 133784
[details]
Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Bernhard Bauer
Comment 13
2012-03-27 01:13:49 PDT
Created
attachment 133997
[details]
Patch
Adam Barth
Comment 14
2012-03-27 01:16:46 PDT
> I still need to clean up the fallback code.
In the future, please create a new bug for a new patch. Feel free to post a link to the new bug in the old bug.
Bernhard Bauer
Comment 15
2012-03-27 01:17:52 PDT
(In reply to
comment #14
)
> > I still need to clean up the fallback code. > > In the future, please create a new bug for a new patch. Feel free to post a link to the new bug in the old bug.
OK, sorry.
Adam Barth
Comment 16
2012-03-27 01:22:57 PDT
Comment on
attachment 133997
[details]
Patch Certainly not a big deal. :) Do we need to update
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS
before landing this patch?
Bernhard Bauer
Comment 17
2012-03-27 01:52:13 PDT
(In reply to
comment #16
)
> (From update of
attachment 133997
[details]
) > Certainly not a big deal. :) > > Do we need to update
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS
before landing this patch?
I'm not sure, actually. The string changes landed in
http://crrev.com/128891
, which is after the revision in DEPS (
http://crrev.com/128426
), but the test expectations already have the new strings (
http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-mac-snowleopard/compositing/plugins/composited-plugin-expected.png
, for example). Or is WebKit DEPS only used for building, not for layout tests?
Adam Barth
Comment 18
2012-03-27 01:55:47 PDT
> Or is WebKit DEPS only used for building, not for layout tests?
It's used in webkit.org-based builds, but we use chromium.org-based builds to run the LayoutTests. It sounds like we need to update the DEPS file. (It's usually easy, just make sure everything compiles with the larger number.)
Bernhard Bauer
Comment 19
2012-03-27 02:52:41 PDT
(In reply to
comment #18
)
> > Or is WebKit DEPS only used for building, not for layout tests? > > It's used in webkit.org-based builds, but we use chromium.org-based builds to run the LayoutTests. It sounds like we need to update the DEPS file. (It's usually easy, just make sure everything compiles with the larger number.)
I updated the DEPS file locally, and ran update-webkit --chromium and build-webkit --chromium, which succeeded. Do we roll DEPS in a separate patch, or the same?
Adam Barth
Comment 20
2012-03-27 13:47:57 PDT
> Do we roll DEPS in a separate patch, or the same?
Usually a separate patch.
Bernhard Bauer
Comment 21
2012-03-29 01:33:53 PDT
(In reply to
comment #20
)
> > Do we roll DEPS in a separate patch, or the same? > > Usually a separate patch.
OK, it seems
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS
has been rolled past
http://crrev.com/128891
(after I forgot about it yesterday :-D ) Are we good to land the cleanup now?
Bernhard Bauer
Comment 22
2012-04-13 06:52:11 PDT
(In reply to
comment #21
)
> (In reply to
comment #20
) > > > Do we roll DEPS in a separate patch, or the same? > > > > Usually a separate patch. > > OK, it seems
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS
has been rolled past
http://crrev.com/128891
(after I forgot about it yesterday :-D ) > > Are we good to land the cleanup now?
Adam, ping? I don't think I can commit the patch myself.
WebKit Review Bot
Comment 23
2012-04-13 17:27:54 PDT
Comment on
attachment 133997
[details]
Patch Clearing flags on attachment: 133997 Committed
r114186
: <
http://trac.webkit.org/changeset/114186
>
WebKit Review Bot
Comment 24
2012-04-13 17:28:12 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