rename variables from protection to safebrowsing or urlclassifier

RESOLVED FIXED

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: Tony Chang (Google), Assigned: Tony Chang (Google))

Tracking

({fixed1.8.1})

Trunk
x86
Linux
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

44.51 KB, patch
Brett Wilson
: review+
Ben Goodger (use ben at mozilla dot org for email)
: approval-branch-1.8.1+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
After the directory rename (bug 335032), we need to rename the paths in the toolkit.jar file as well and the name of many of the objects.
(Assignee)

Comment 1

12 years ago
Created attachment 219906 [details] [diff] [review]
renaming variables

Renaming variables to avoid confusion.  We should also rename the files, but that will happen in a different CL so it's clear what's actually changing.
Attachment #219906 - Flags: review?(brettw)

Comment 2

12 years ago
Comment on attachment 219906 [details] [diff] [review]
renaming variables

Looks OK, except you should rename the .idl files to correspond to the new interface names.
Attachment #219906 - Flags: review?(brettw) → review-
(Assignee)

Comment 3

12 years ago
Can I do that in a separate change so the edits are easy to distinguish from the file renames?

Comment 4

12 years ago
(In reply to comment #3)
> Can I do that in a separate change so the edits are easy to distinguish from
> the file renames?

OK. I'll check this in in a minute.

Comment 5

12 years ago
Comment on attachment 219906 [details] [diff] [review]
renaming variables

(file renames to be in next patch)
Attachment #219906 - Flags: review- → review+
(Assignee)

Updated

12 years ago
Attachment #219906 - Flags: approval-branch-1.8.1?(bugs)
Comment on attachment 219906 [details] [diff] [review]
renaming variables

> PROT_GlobalStore.getAppDirectoryName = function() {
>-  return PROT_GlobalStore.getPref_("protection.datadirectory");
>+  return PROT_GlobalStore.getPref_("safebrowsing.datadirectory");

If the name of this is "urlclassifier" and not "protection" then why is this "safebrowsing.datadirectory" and not "urlclassifier.datadirectory"?
Attachment #219906 - Flags: superreview-
Attachment #219906 - Flags: approval-branch-1.8.1?(bugs)
Attachment #219906 - Flags: approval-branch-1.8.1-
(Assignee)

Comment 7

12 years ago
(In reply to comment #6)
> If the name of this is "urlclassifier" and not "protection" then why is this
> "safebrowsing.datadirectory" and not "urlclassifier.datadirectory"?
> 

There could be multiple objects that use the urlclassifier object.  Each may want it's own data directory. It also makes sense to group all the safebrowsing related prefs together.

I don't think it's a big deal either way, so if you want me to change it, I will.
Wouldn't they need to create a copy of globalstore.js then to change that? If that's not the end of the world, doesn't that imply that globalstore.js is a file that should live in browser/components/safe-browsing instead?

If you are thinking hypothetically, I'd say just rename to urlclassifier. Otherwise I think you need to come up with a better solution...
(Assignee)

Comment 9

12 years ago
Created attachment 219930 [details] [diff] [review]
renaming variables

Renamed to urlclassifier.datadirectory as suggested.
Attachment #219906 - Attachment is obsolete: true
Attachment #219930 - Flags: review?(bugs)
Comment on attachment 219906 [details] [diff] [review]
renaming variables

Tony said he'd move this to browser/ later
Attachment #219906 - Attachment is obsolete: false
Attachment #219906 - Flags: superreview-
Attachment #219906 - Flags: approval-branch-1.8.1-
Attachment #219906 - Flags: approval-branch-1.8.1+
Attachment #219930 - Flags: review?(bugs)
(Assignee)

Updated

12 years ago
Attachment #219930 - Attachment is obsolete: true

Comment 11

12 years ago
Fixed on branch and trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.