Closed
Bug 799396
Opened 12 years ago
Closed 12 years ago
Use Map instead of object literal in browser-plugins.js for hash tables
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 19
People
(Reporter: jaws, Assigned: jaws)
References
Details
Attachments
(1 file, 2 obsolete files)
15.19 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
The object literal that is used to track missingPlugins as well as the list of the plugins on a page currently use object literals. If a plugin were to have the name of "__proto__" this could cause some really odd bugs to crop up. I've converted the usages of these object literals to use Map, which won't suffer from these issues.
Attachment #669414 -
Flags: review?(dao)
Attachment #669414 -
Flags: feedback?(dkeeler)
Comment 1•12 years ago
|
||
Is it possible for a mime type to be "__proto__" and still reach this front-end code?
Comment 2•12 years ago
|
||
We already know it's possible for the mime type to be "9000" (bug 797677) here, so it's likely that "__proto__" or "watch" can reach here as well. Fwiw: > Object.getOwnPropertyNames(Object.prototype) constructor,toSource,toString,toLocaleString,valueOf,watch,unwatch,hasOwnProperty,isPrototypeOf,propertyIsEnumerable,__defineGetter__,__defineSetter__,__lookupGetter__,__lookupSetter__
Comment on attachment 669414 [details] [diff] [review] Patch Review of attachment 669414 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #669414 -
Flags: feedback?(dkeeler) → feedback+
Updated•12 years ago
|
Attachment #669414 -
Flags: feedback+
Comment 4•12 years ago
|
||
Comment on attachment 669414 [details] [diff] [review] Patch >--- a/toolkit/mozapps/plugins/content/pluginInstallerWizard.js >+++ b/toolkit/mozapps/plugins/content/pluginInstallerWizard.js >@@ -30,19 +30,19 @@ function nsPluginInstallerWizard(){ > this.mNeedsRestart = false; > > // arguments[0] is an array that contains two items: > // an array of mimetypes that are missing This comment needs an update. > // a reference to the browser that needs them, > // so we can notify which browser can be reloaded. > > if ("arguments" in window) { >- for (var item in window.arguments[0].plugins){ >- this.mPluginRequestArray[window.arguments[0].plugins[item].mimetype] = >- new nsPluginRequest(window.arguments[0].plugins[item]); >+ for (let [item, pluginInfo] of window.arguments[0].plugins){ >+ this.mPluginRequestArray[pluginInfo.mimetype] = >+ new nsPluginRequest(pluginInfo); "item" is actually the mime type here, if I understand correctly. Please rename it accordingly and use it instead of pluginInfo.mimetype.
Attachment #669414 -
Flags: review?(dao) → review+
Assignee | ||
Comment 5•12 years ago
|
||
While fixing the nits I noticed another object in pluginInstallerWizard.js that could be converted to a Map.
Attachment #669414 -
Attachment is obsolete: true
Attachment #669826 -
Flags: review?(dao)
Comment 6•12 years ago
|
||
Comment on attachment 669826 [details] [diff] [review] Patch v1.1 This is the same patch as the previous one.
Attachment #669826 -
Flags: review?(dao)
Assignee | ||
Comment 7•12 years ago
|
||
Sorry about that, forgot to qref.
Attachment #669826 -
Attachment is obsolete: true
Attachment #669830 -
Flags: review?(dao)
Comment 8•12 years ago
|
||
Comment on attachment 669830 [details] [diff] [review] Patch v1.1 (qref'd) >+ // a map of (key=mimetype, value=pluginInfo object) that are missing, "a mimetype->pluginInfo map of missing plugins"
Attachment #669830 -
Flags: review?(dao) → review+
Assignee | ||
Comment 9•12 years ago
|
||
Thanks. https://hg.mozilla.org/integration/mozilla-inbound/rev/c144e89952c1
Flags: in-testsuite+
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c144e89952c1
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in
before you can comment on or make changes to this bug.
Description
•