Closed
Bug 361203
Opened 17 years ago
Closed 17 years ago
Suiterunner: More Information button doesn't work on Smart Browsing pane.
Categories
(SeaMonkey :: Preferences, defect)
SeaMonkey
Preferences
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file, 2 obsolete files)
5.32 KB,
patch
|
kairo
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
On the preferences window of suiterunner, the Smart Browsing pane has a "More Information" button which doesn't do anything. For normal SeaMonkey builds it works fine. The problem is that smartBrowsingURL (http://lxr.mozilla.org/seamonkey/search?string=smartBrowsingURL) is currently defined in region.properties which suiterunner is dropping. What needs to happen is that prefs should use the urlformatter to get the URL, with the URL being placed in brand.properties.
Assignee | ||
Comment 1•17 years ago
|
||
This is work in progress. I thought I would not use the url formatter as its not really necessary/appropriate in this case, but I'm still moving the string to brand.properties. I also thought I'd use the new openAsExternal function that Ian wrote from contentAreaUtils.js - I've had two problems: 1) pref is not defined (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/contentAreaUtils.js&rev=1.142&mark=100#98) - for now I've defined it locally, but should the function get the pref branch itself (the mailnews instance has pref defined globally)? 2) with the pref defined globally (as per the patch) I then get: JavaScript error: chrome://communicator/content/contentAreaUtils.js, line 73: contentFrame has no properties http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/contentAreaUtils.js&rev=1.142&mark=73#70 Is there a sensible way around that considering I'm in a pref window and not a browser?
Comment 2•17 years ago
|
||
(In reply to comment #1) >Is there a sensible way around that considering I'm in a pref window and not a browser? Not currently, but I'm supposed to replace urlSecurityCheck because it has a security flaw.
Assignee | ||
Comment 3•17 years ago
|
||
(In reply to comment #2) > (In reply to comment #1) > >Is there a sensible way around that considering I'm in a pref window and not a browser? > Not currently, but I'm supposed to replace urlSecurityCheck because it has a > security flaw. Maybe I'll leave it as the current implementation for opening the link and we could pick it up later if we want to - especially as I think there will be other cases we need to look at.
![]() |
||
Comment 4•17 years ago
|
||
(In reply to comment #1) > This is work in progress. I thought I would not use the url formatter as its > not really necessary/appropriate in this case, but I'm still moving the string > to brand.properties. This _is_ a URL though, so I actually think using the urlformatter would be the correct way here.
Assignee | ||
Comment 5•17 years ago
|
||
Revised to use the urlformatter and just use the open window function as previous implementation.
Assignee: prefs → bugzilla
Attachment #249038 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #249909 -
Flags: superreview?(neil)
Attachment #249909 -
Flags: review?(neil)
Comment 6•17 years ago
|
||
Comment on attachment 249909 [details] [diff] [review] Patch v1 Despite comment #4 I think KaiRo should officially grant l10n approval on this one.
Attachment #249909 -
Flags: review?(neil) → review?(kairo)
Comment 7•17 years ago
|
||
Comment on attachment 249909 [details] [diff] [review] Patch v1 [So, what are the rules on when we use a property and when we format a localised pref?]
Attachment #249909 -
Flags: superreview?(neil) → superreview+
![]() |
||
Comment 8•17 years ago
|
||
I actually think the best idea is to let all URLs go through the full way of url formatter, i.e. making them localized prefs. This probably also would make that code cleaner as it wouldn't need to include the bundle directly, as that will be done by the prefs. (I didn't even know we could feed something that's not a pref to urlformatter...)
![]() |
||
Comment 9•17 years ago
|
||
Comment on attachment 249909 [details] [diff] [review] Patch v1 r- as Mark agrees with me on IRC - let's make all such URLs localized prefs when we run across them. 1) This fits better with the style used by other toolkit apps, as they use pref'd URLs everywhere (simple prefs, not localized ones like us) 2) Additionally, using prefs makes it easier for somone to tailor SeaMonkey to corporate/organisation needs by just shipping the "right" prefs file(s) to their computers. (Of course, from that POV, we should pref all URLs we have - we may argue at some point if all of them need to be localized, but for now, I think we should take them to brand.properties)
Attachment #249909 -
Flags: review?(kairo) → review-
Assignee | ||
Comment 10•17 years ago
|
||
Revised version that uses a localized pref.
Attachment #249909 -
Attachment is obsolete: true
Attachment #250071 -
Flags: review?(kairo)
![]() |
||
Comment 11•17 years ago
|
||
Comment on attachment 250071 [details] [diff] [review] Patch v2 Looks good now, r=me
Attachment #250071 -
Flags: review?(kairo) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #250071 -
Flags: superreview?(neil)
Comment 12•17 years ago
|
||
Comment on attachment 250071 [details] [diff] [review] Patch v2 I think browser.related. is misleading - What's Related is a sidebar panel completely unrelated (!) to Smart Browsing. How about keyword.moreInfoURL?
Attachment #250071 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 13•17 years ago
|
||
(In reply to comment #12) > (From update of attachment 250071 [details] [diff] [review]) > I think browser.related. is misleading - What's Related is a sidebar panel > completely unrelated (!) to Smart Browsing. How about keyword.moreInfoURL? > Yep, much better, I checked this in with keyword.moreInfoURL.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•