Closed
Bug 720310
Opened 13 years ago
Closed 13 years ago
Rename BrowserSetForcedDetector and remove doReload parameter
Categories
(Firefox :: Menus, defect)
Firefox
Menus
Tracking
()
RESOLVED
FIXED
Firefox 12
People
(Reporter: smontagu, Assigned: smontagu)
References
Details
Attachments
(1 file, 2 obsolete files)
5.02 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
Follow-up to bug 713825 -- see bug 713825 comment 7: "Seems like BrowserSetForcedDetector should be renamed and any calls with !doReload be eliminated. Followup bug?"
Attachment #590644 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #590644 -
Flags: review? → review?(gavin.sharp)
Comment 1•13 years ago
|
||
Comment on attachment 590644 [details] [diff] [review]
Patch
>--- a/toolkit/content/charsetOverlay.js
>+++ b/toolkit/content/charsetOverlay.js
>@@ -1,14 +1,14 @@
> function MultiplexHandler(aEvent)
> {
> MultiplexHandlerEx(
> aEvent,
> function Browser_SelectDetector(event) {
>- BrowserSetForcedDetector(true/*Reload from history*/);
>+ BrowserCharsetReload(/*Reload from history*/);
> /* window.content.location.reload() will re-download everything */
Nit: Maybe merge those two comments and put them above BrowserCharsetReload()?
Comment 2•13 years ago
|
||
In browser.js, why don't you just call BrowserCharsetReload() directly, and avoid adding a useless CharsetReload()?
Same question applies to [Browser]SetForcedCharset.
Assignee | ||
Comment 3•13 years ago
|
||
Fine with me -- I thought maybe there was some esoteric reason why it was done that way.
Assignee: nobody → smontagu
Attachment #590644 -
Attachment is obsolete: true
Attachment #590644 -
Flags: review?(gavin.sharp)
Attachment #590815 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 4•13 years ago
|
||
With qrefresh this time
Attachment #590815 -
Attachment is obsolete: true
Attachment #590815 -
Flags: review?(gavin.sharp)
Attachment #590816 -
Flags: review?(gavin.sharp)
Comment 5•13 years ago
|
||
This code is too old and crufty to give it any benefit of the doubt :)
Comment 6•13 years ago
|
||
Comment on attachment 590816 [details] [diff] [review]
Patch v.2 (no, really!)
>diff --git a/toolkit/content/charsetOverlay.js b/toolkit/content/charsetOverlay.js
> function MultiplexHandler(aEvent)
>+ BrowserCharsetReload(/*Reload from history*/);
The comment is no longer needed. You'll also need to update comm-central's suite/browser/browser.js accordingly.
Attachment #590816 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6)
> Comment on attachment 590816 [details] [diff] [review]
> You'll also need to update comm-central's suite/browser/browser.js accordingly.
That would be bug 720312
Assignee | ||
Comment 8•13 years ago
|
||
Target Milestone: --- → Firefox 12
Assignee | ||
Comment 9•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•