Remove "pluginsPage" from _getPluginInfo because it is unused

RESOLVED FIXED in mozilla24

Status

()

Core
Plug-ins
P2
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Currently _getPluginInfo spends some effort to get the correct pluginsPage for a plugin instance. As far as I can tell, this information is unused. As part of the CtP doorhanger work I removed it, because I have other things to put in this function and it was getting confused. And removing dead code is good!

cers, you don't know of any clients of this property related to your recent PFS work, do you?
(Assignee)

Updated

5 years ago
Flags: needinfo?(csonne)
Priority: -- → P2
The only remaining reference to pluginsPage is in

http://dxr.mozilla.org/mozilla-central/toolkit/mozapps/plugins/content/pluginInstallerWizard.js

There was also the confusingly similar-but-different pluginUrl, removed in bug 548133
Actually, if you properly search without case sensitivity, we normalize this URL in nsPluginInstanceOwner::FixUpURLS for some reason. (This probably wont survive bug 853995)
(Assignee)

Comment 3

5 years ago
Created attachment 752195 [details] [diff] [review]
remove _pluginsPage, rev. 1

I didn't search far enough afield ;-) I think we can safely break pluginsPage in the PFS UI right now, if that's ok.
Attachment #752195 - Flags: review?(jaws)
Attachment #752195 - Flags: feedback?(csonne)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #0)
> cers, you don't know of any clients of this property related to your recent
> PFS work, do you?

At least after bug 836420, I'm quite certain it's not used anywhere.
Flags: needinfo?(csonne)
I think it'd be safer to wait until bug 836420 is fixed.
Status: NEW → ASSIGNED
Depends on: 836420
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 6

5 years ago
When will that be done? I've got a bunch of work sitting on top of this that has to make Fx24.
Flags: needinfo?(csonne)
Comment on attachment 752195 [details] [diff] [review]
remove _pluginsPage, rev. 1

Review of attachment 752195 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think PFS is going to be removed in Fx24. Dolske, are you OK with breaking this part of the PFS UI?
Attachment #752195 - Flags: review?(jaws)
Attachment #752195 - Flags: review?(dolske)
Attachment #752195 - Flags: feedback?(cers)
Attachment #752195 - Flags: feedback+
Comment on attachment 752195 [details] [diff] [review]
remove _pluginsPage, rev. 1

Review of attachment 752195 [details] [diff] [review]:
-----------------------------------------------------------------

bye-bye!

I suppose we could nuke the usage in pluginInstallerWizard.js too, but that code is already old and nasty and should just die.
Attachment #752195 - Flags: review?(dolske) → review+

Comment 10

5 years ago
Oh, bug 655916 is already fixed? Awesome.
(Assignee)

Comment 11

4 years ago
This landed rolled up with other CtP patches.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Target Milestone: --- → mozilla24
(Assignee)

Updated

4 years ago
Flags: needinfo?(cers)
You need to log in before you can comment on or make changes to this bug.