Last Comment Bug 783025 - Replace String.indexOf(...) != -1 with String.contains(...) in browser/base/
: Replace String.indexOf(...) != -1 with String.contains(...) in browser/base/
Status: RESOLVED FIXED
[good first bug][mentor=dao][lang=js]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 18
Assigned To: Amod [:AMoz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-15 10:57 PDT by Dão Gottwald [:dao]
Modified: 2012-09-17 04:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
diff file of aboutSyncTab.js (1.09 KB, patch)
2012-08-22 11:19 PDT, Amod [:AMoz]
no flags Details | Diff | Review
corrected diff file of aboutSyncTab.js (1.09 KB, patch)
2012-08-23 08:51 PDT, Amod [:AMoz]
no flags Details | Diff | Review
it is a monolithic patch of all the 17 files mentioned (15.73 KB, patch)
2012-08-31 03:05 PDT, Amod [:AMoz]
no flags Details | Diff | Review
rectified errors ! (7.67 KB, patch)
2012-08-31 09:19 PDT, Amod [:AMoz]
no flags Details | Diff | Review
single patch of all files. (14.97 KB, patch)
2012-08-31 12:07 PDT, Amod [:AMoz]
no flags Details | Diff | Review
patch. (14.40 KB, patch)
2012-08-31 13:47 PDT, Amod [:AMoz]
dao+bmo: feedback+
Details | Diff | Review
edited final as mentioned in comment 24. (4.24 KB, patch)
2012-09-08 03:21 PDT, Amod [:AMoz]
no flags Details | Diff | Review
ptach of all the files !!! (13.51 KB, patch)
2012-09-08 06:37 PDT, Amod [:AMoz]
no flags Details | Diff | Review
patch ! (12.95 KB, patch)
2012-09-08 10:21 PDT, Amod [:AMoz]
no flags Details | Diff | Review
patch ! (12.93 KB, patch)
2012-09-08 11:35 PDT, Amod [:AMoz]
dao+bmo: review+
Details | Diff | Review

Description Dão Gottwald [:dao] 2012-08-15 10:57:41 PDT
Since bug 772733 landed, 'foo.indexOf(bar) != -1' can be simplified to 'foo.contains(bar)'.

This only applies to strings, not arrays.

Here's a list of candidates:
http://mxr.mozilla.org/mozilla-central/search?string=.indexOf%28&case=1&find=%2Fbrowser%2Fbase%2F&filter=-1
Comment 1 Amod [:AMoz] 2012-08-16 09:16:10 PDT
Sir, I would like to contribute to this bug..
Comment 2 Amod [:AMoz] 2012-08-16 09:24:36 PDT
Sir, I would like to contribute to this bug. I would be highly obliged if u assign the bug to me...Will u explain the bug in detail ?
Comment 3 Dão Gottwald [:dao] 2012-08-16 09:55:47 PDT
Do you already have a local copy of the Firefox code base?
Comment 4 Amod [:AMoz] 2012-08-17 10:42:34 PDT
no..had prob in linux. so after reinstalling linux..i am currently downloading the source code.. but i am confused about what exactly the bug is..
Comment 5 Dão Gottwald [:dao] 2012-08-17 10:47:13 PDT
Code is currently using indexOf to check whether a string contains a sub-string. There is no real "bug", i.e. that code should already be working correctly. But it can be simplified, because we have the new 'contains' string method. We should use that instead of indexOf.
Comment 6 Amod [:AMoz] 2012-08-19 05:57:02 PDT
I have downloaded the entire source code. Build took 5 hrs, So sir u can tell me which file has the source code concerned to the bug and what exactly I need to do.
Comment 7 Dão Gottwald [:dao] 2012-08-19 06:14:46 PDT
The 'indexOf' pattern appears in multiple files. They are listed here:
http://mxr.mozilla.org/mozilla-central/search?string=.indexOf%28&case=1&find=%2Fbrowser%2Fbase%2F&filter=-1

However, some of these search results are for arrays. You'll need to figure out where 'indexOf' is used on a string and replace it with 'contains' there.
Comment 8 Amod [:AMoz] 2012-08-22 11:19:14 PDT
Created attachment 654281 [details] [diff] [review]
diff file of aboutSyncTab.js

i have uploaded the patch for /browser/base/content/sync/aboutSyncTabs.js.

In /browser/base/content/newtab/updater.js, aSites is the array of sites remaining in the grid. so i dint replace the indexOf() with contains()..

I am new to this environment hence i apologise for any mistake.
Comment 9 Amod [:AMoz] 2012-08-22 11:21:21 PDT
if my approach is appropriate for this file then i will proceed for the rest of the files.
Comment 10 Dão Gottwald [:dao] 2012-08-22 15:44:14 PDT
Comment on attachment 654281 [details] [diff] [review]
diff file of aboutSyncTab.js

>-        if (item.getAttribute("url").toLowerCase().indexOf(val) == -1 &&
>-            item.getAttribute("title").toLowerCase().indexOf(val) == -1)
>+        if (item.getAttribute("url").toLowerCase().contains(val) == -1 &&
>+            item.getAttribute("title").toLowerCase().contains(val) == -1)

indexOf returns an integer (-1 if the substring wasn't found). contains returns a boolean (false if the substring wasn't found). So you need to replace this:

A.indexOf(B) == -1

with this:

!A.contains(B)
Comment 11 Amod [:AMoz] 2012-08-23 08:51:07 PDT
Created attachment 654643 [details] [diff] [review]
corrected diff file of aboutSyncTab.js

extremely sorry Sir...i have attached patch for /browser/base/content/sync/aboutSyncTabs.js.
sir.. like wise there are 28 candidates.. So shall i upload patch for every file..i think that would be a cumbersome in terms of viewing later..
Comment 12 Dão Gottwald [:dao] 2012-08-23 09:00:29 PDT
Comment on attachment 654643 [details] [diff] [review]
corrected diff file of aboutSyncTab.js

This looks good codewise. The formatting isn't correct, though. You should use spaces rather than tab stops and keep the indentation like it was before.
Comment 13 Dão Gottwald [:dao] 2012-08-23 09:02:41 PDT
(In reply to Amod from comment #11)
> Created attachment 654643 [details] [diff] [review]
> corrected diff file of aboutSyncTab.js
> 
> extremely sorry Sir...i have attached patch for
> /browser/base/content/sync/aboutSyncTabs.js.
> sir.. like wise there are 28 candidates.. So shall i upload patch for every
> file..i think that would be a cumbersome in terms of viewing later..

One monolithic patch would be preferable.
Comment 14 Amod [:AMoz] 2012-08-31 03:05:53 PDT
Created attachment 657204 [details] [diff] [review]
it is a monolithic patch of all the 17 files mentioned

I have made changes in the files..Some candidates were arrays, so I didn't make any changes.
Comment 15 Amod [:AMoz] 2012-08-31 03:08:01 PDT
Sir, I will make necessary changes in indentations once you approve the files correct as codewise.
Comment 16 Dão Gottwald [:dao] 2012-08-31 04:01:06 PDT
Comment on attachment 657204 [details] [diff] [review]
it is a monolithic patch of all the 17 files mentioned

>--- a/browser/base/content/browser-places.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/browser-places.js	Fri Aug 31 02:52:37 2012 +0000
>@@ -977,17 +977,17 @@ var PlacesStarButton = {
>         Components.utils.reportError("PlacesStarButton did not receive current URI");
>         return;
>       }
> 
>       // It's possible that onItemAdded gets called before the async statement
>       // calls back.  For such an edge case, retain all unique entries from both
>       // arrays.
>       this._itemIds = this._itemIds.filter(
>-        function (id) aItemIds.indexOf(id) == -1
>+        function (id) !aItemIds.contains(id)
>       ).concat(aItemIds);

aItemIds is an array.

>@@ -1031,17 +1031,17 @@ var PlacesStarButton = {
>   function PSB_onItemAdded(aItemId, aFolder, aIndex, aItemType, aURI)
>   {
>     if (!this._starIcon) {
>       return;
>     }
> 
>     if (aURI && aURI.equals(this._uri)) {
>       // If a new bookmark has been added to the tracked uri, register it.
>-      if (this._itemIds.indexOf(aItemId) == -1) {
>+      if (!this._itemIds.indexOf(aItemId)) {

This change is wrong, the return value of indexOf still needs to be compared to -1.

>--- a/browser/base/content/browser.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/browser.js	Fri Aug 31 02:52:37 2012 +0000
>@@ -1043,17 +1043,17 @@ var gBrowserInit = {
>           .QueryInterface(Ci.nsIInterfaceRequestor)
>           .getInterface(Ci.nsIXULWindow)
>           .XULBrowserWindow = window.XULBrowserWindow;
>     window.QueryInterface(Ci.nsIDOMChromeWindow).browserDOMWindow =
>       new nsBrowserAccess();
> 
>     // set default character set if provided
>     if ("arguments" in window && window.arguments.length > 1 && window.arguments[1]) {
>-      if (window.arguments[1].indexOf("charset=") != -1) {
>+      if (window.arguments[1].contains("charset=")) {

The original code isn't quite correct, it should have checked whether ...indexOf("charset=") is 0. Similarly, we should use "startsWith" rather than "contains":

      if (window.arguments[1].startsWith("charset=")) {

>@@ -3485,17 +3485,17 @@ function FillHistoryMenu(aParent) {
> 
>     aParent.appendChild(item);
>   }
>   return true;
> }
> 
> function addToUrlbarHistory(aUrlToAdd) {
>   if (aUrlToAdd &&
>-      aUrlToAdd.indexOf(" ") == -1 &&
>+      aUrlToAdd.contains(" ") &&

this should be !aUrlToAdd.contains(" ")

>--- a/browser/base/content/tabbrowser.xml	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/tabbrowser.xml	Fri Aug 31 02:52:37 2012 +0000
>@@ -1804,24 +1804,24 @@
>                 return !tab.closing;
>               }, this);
>             }
> 
>             // Try to find a remaining tab that comes after the given tab
>             var tab = aTab;
>             do {
>               tab = tab.nextSibling;
>-            } while (tab && remainingTabs.indexOf(tab) == -1);
>+            } while (tab && !remainingTabs.contains(tab));
> 
>             if (!tab) {
>               tab = aTab;
> 
>               do {
>                 tab = tab.previousSibling;
>-              } while (tab && remainingTabs.indexOf(tab) == -1);
>+              } while (tab && !remainingTabs.contains(tab));

remainingTabs is an array.

>@@ -2011,17 +2011,17 @@
>         </body>
>       </method>
> 
>       <method name="showOnlyTheseTabs">
>         <parameter name="aTabs"/>
>         <body>
>         <![CDATA[
>           Array.forEach(this.tabs, function(tab) {
>-            if (aTabs.indexOf(tab) == -1)
>+            if (!aTabs.contains(tab))

aTabs is an array.

>--- a/browser/base/content/test/browser_bug555224.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/test/browser_bug555224.js	Fri Aug 31 02:52:37 2012 +0000
>@@ -14,17 +14,17 @@ function afterZoomAndLoad(aCallback, aTa
>       executeSoon(aCallback);
>   }, true);
>   let oldAPTS = FullZoom._applyPrefToSetting;
>   FullZoom._applyPrefToSetting = function(value, browser) {
>     if (!value)
>       value = undefined;
>     oldAPTS.call(FullZoom, value, browser);
>     // Don't reset _applyPrefToSetting until we've seen the about:blank load(s)
>-    if (browser && (browser.currentURI.spec.indexOf("http:") != -1)) {
>+    if (browser && browser.currentURI.spec.contains("http:")) {

We should also use startsWith here.

>--- a/browser/base/content/test/browser_popupNotification.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/test/browser_popupNotification.js	Fri Aug 31 02:52:37 2012 +0000
>@@ -751,17 +751,17 @@ function triggerSecondaryCommand(popup, 
>     for (let i = 0; i <= index; i++)
>       EventUtils.synthesizeKey("VK_DOWN", {});
> 
>     // Activate
>     EventUtils.synthesizeKey("VK_ENTER", {});
>   }, false);
> 
>   // One down event to open the popup
>-  EventUtils.synthesizeKey("VK_DOWN", { altKey: (navigator.platform.indexOf("Mac") == -1) });
>+  EventUtils.synthesizeKey("VK_DOWN", { altKey: (!navigator.platform.contains("Mac")) });

You can remove the parentheses around !navigator.platform.contains("Mac").
Comment 17 Amod [:AMoz] 2012-08-31 09:19:30 PDT
Created attachment 657319 [details] [diff] [review]
rectified errors !

Sir, i am still confused to distinguish between array and string. I tried to use function typeOfx=="string" but was not sure of how to run the script.
Comment 18 Dão Gottwald [:dao] 2012-08-31 09:24:35 PDT
In the latest patch you seem to have reverted changes that were fine in the previous patch, such as the one in browser-tabPreviews.js.
Comment 19 Amod [:AMoz] 2012-08-31 09:33:26 PDT
Sir, but the latest patch which I uploaded now don't highlight any changes in browser-tabPreivew.js
Comment 20 Dão Gottwald [:dao] 2012-08-31 10:33:29 PDT
(In reply to Amod from comment #19)
> Sir, but the latest patch which I uploaded now don't highlight any changes
> in browser-tabPreivew.js

Yes, but it should! :) Those changes were correct.
Comment 21 Amod [:AMoz] 2012-08-31 12:07:04 PDT
Created attachment 657386 [details] [diff] [review]
single patch of all files.

Sorry Sir. it seems that I had forgot to include this file in patch.
Comment 22 Dão Gottwald [:dao] 2012-08-31 12:48:12 PDT
Comment on attachment 657386 [details] [diff] [review]
single patch of all files.

>--- a/browser/base/content/browser-places.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/browser-places.js	Fri Aug 31 12:03:53 2012 +0000
>@@ -1031,17 +1031,17 @@ var PlacesStarButton = {
>   function PSB_onItemAdded(aItemId, aFolder, aIndex, aItemType, aURI)
>   {
>     if (!this._starIcon) {
>       return;
>     }
> 
>     if (aURI && aURI.equals(this._uri)) {
>       // If a new bookmark has been added to the tracked uri, register it.
>-      if (this._itemIds.indexOf(aItemId) == -1) {
>+      if (!this._itemIds.contains(aItemId)) {
>         this._itemIds.push(aItemId);

this._itemIds is an array.
Comment 23 Amod [:AMoz] 2012-08-31 13:47:35 PDT
Created attachment 657431 [details] [diff] [review]
patch.
Comment 24 Dão Gottwald [:dao] 2012-08-31 19:38:19 PDT
Comment on attachment 657431 [details] [diff] [review]
patch.

Alright, this should be correct now. Now some nitpicking:

>--- a/browser/base/content/browser-places.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/browser-places.js	Fri Aug 31 13:46:15 2012 +0000
>@@ -977,17 +977,17 @@ var PlacesStarButton = {
>         Components.utils.reportError("PlacesStarButton did not receive current URI");
>         return;
>       }
> 
>       // It's possible that onItemAdded gets called before the async statement
>       // calls back.  For such an edge case, retain all unique entries from both
>       // arrays.
>       this._itemIds = this._itemIds.filter(
>-        function (id) aItemIds.indexOf(id) == -1
>+      function (id) aItemIds.indexOf(id) == -1

please restore the original indentation

>--- a/browser/base/content/sync/aboutSyncTabs.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/sync/aboutSyncTabs.js	Fri Aug 31 13:46:15 2012 +0000
>@@ -52,19 +52,18 @@ let RemoteTabViewer = {
>     let val = event.target.value.toLowerCase();
>     let numTabs = this._tabsList.getRowCount();
>     let clientTabs = 0;
>     let currentClient = null;
>     for (let i = 0;i < numTabs;i++) {
>       let item = this._tabsList.getItemAtIndex(i);
>       let hide = false;
>       if (item.getAttribute("type") == "tab") {
>-        if (item.getAttribute("url").toLowerCase().indexOf(val) == -1 &&
>-            item.getAttribute("title").toLowerCase().indexOf(val) == -1)
>-          hide = true;
>+	if(!item.getAttribute("url").toLowerCase().contains(val) && !item.getAttribute("title").toLowerCase().contains(val))
>+	hide = true;

ditto

>--- a/browser/base/content/tabbrowser.xml	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/tabbrowser.xml	Fri Aug 31 13:46:15 2012 +0000
>@@ -2011,17 +2011,17 @@
>         </body>
>       </method>
> 
>       <method name="showOnlyTheseTabs">
>         <parameter name="aTabs"/>
>         <body>
>         <![CDATA[
>           Array.forEach(this.tabs, function(tab) {
>-            if (aTabs.indexOf(tab) == -1)
>+            if(aTabs.indexOf(tab) == -1)

please add back the removed space
Comment 25 Dão Gottwald [:dao] 2012-09-07 07:35:49 PDT
Comment on attachment 657431 [details] [diff] [review]
patch.

This should be good to go once comment 24 is addressed.
Comment 26 Amod [:AMoz] 2012-09-07 11:29:54 PDT
Yes Sir, since i had very much problem with my system, i was not able to attend to it. Now i am back and will solve it within 1-2 days.
Comment 27 Amod [:AMoz] 2012-09-08 03:21:57 PDT
Created attachment 659468 [details] [diff] [review]
edited final as mentioned in comment 24.
Comment 28 Dão Gottwald [:dao] 2012-09-08 03:25:59 PDT
Comment on attachment 659468 [details] [diff] [review]
edited final as mentioned in comment 24.

We need a complete patch like attachment 657431 [details] [diff] [review], with the changes from comment 24 incorporated.
Comment 29 Amod [:AMoz] 2012-09-08 06:37:09 PDT
Created attachment 659484 [details] [diff] [review]
ptach of all the files !!!

Due to my system problem i was not able to attend earlier. Sorry for the same. I have uploaded a patch of all the files. If there is still any error then mention it. Thanks !!!
Comment 30 Dão Gottwald [:dao] 2012-09-08 06:57:08 PDT
Comment on attachment 659484 [details] [diff] [review]
ptach of all the files !!!

>--- a/browser/base/content/browser-places.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/browser-places.js	Sat Sep 08 17:32:01 2012 +0530
>@@ -977,17 +977,17 @@ var PlacesStarButton = {
>         Components.utils.reportError("PlacesStarButton did not receive current URI");
>         return;
>       }
> 
>       // It's possible that onItemAdded gets called before the async statement
>       // calls back.  For such an edge case, retain all unique entries from both
>       // arrays.
>       this._itemIds = this._itemIds.filter(
>-        function (id) aItemIds.indexOf(id) == -1
>+      	 function (id) aItemIds.indexOf(id) == -1

You replaced spaces with a tab stop here. Please restore the original indentation using spaces only.

>--- a/browser/base/content/sync/aboutSyncTabs.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/sync/aboutSyncTabs.js	Sat Sep 08 17:32:01 2012 +0530
>@@ -52,19 +52,21 @@ let RemoteTabViewer = {
>     let val = event.target.value.toLowerCase();
>     let numTabs = this._tabsList.getRowCount();
>     let clientTabs = 0;
>     let currentClient = null;
>     for (let i = 0;i < numTabs;i++) {
>       let item = this._tabsList.getItemAtIndex(i);
>       let hide = false;
>       if (item.getAttribute("type") == "tab") {
>-        if (item.getAttribute("url").toLowerCase().indexOf(val) == -1 &&
>-            item.getAttribute("title").toLowerCase().indexOf(val) == -1)
>-          hide = true;
>+	if
>+(!item.getAttribute("url").toLowerCase().contains(val) && 
>+
>+!item.getAttribute("title").toLowerCase().contains(val))
>+	hide = true;

This code should be indented exactly as it was before, i.e. the same amount of spaces, no tab stops and no additional line breaks.

>--- a/browser/base/content/tabbrowser.xml	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/tabbrowser.xml	Sat Sep 08 17:32:01 2012 +0530

>       <method name="showOnlyTheseTabs">
>         <parameter name="aTabs"/>
>         <body>
>         <![CDATA[
>           Array.forEach(this.tabs, function(tab) {
>-            if (aTabs.indexOf(tab) == -1)
>+            if	(aTabs.indexOf(tab) == -1)
>               this.hideTab(tab);
>             else
>               this.showTab(tab);
>           }, this);

You introduced a tab stop here as well.

Also, attachment 657431 [details] [diff] [review] contained a change to browser_popupNotification.js, which you seem to have missed here.
Comment 31 Amod [:AMoz] 2012-09-08 10:21:46 PDT
Created attachment 659502 [details] [diff] [review]
patch !
Comment 32 Dão Gottwald [:dao] 2012-09-08 10:29:37 PDT
Comment on attachment 659502 [details] [diff] [review]
patch !

>--- a/browser/base/content/sync/aboutSyncTabs.js	Sat Aug 18 07:22:18 2012 -0400
>+++ b/browser/base/content/sync/aboutSyncTabs.js	Sat Sep 08 21:18:56 2012 +0530

>-        if (item.getAttribute("url").toLowerCase().indexOf(val) == -1 &&
>-            item.getAttribute("title").toLowerCase().indexOf(val) == -1)
>-          hide = true;
>+	if(!item.getAttribute("url").toLowerCase().contains(val) && 
>+          !item.getAttribute("title").toLowerCase().contains(val))
>+	hide = true;

The first line is indented with a tab stop; should be eight spaces instead.
There should also be a space between "if" and "(".
The second line needs to be indented by more two spaces.
The third line is indented with a tab stop; should be ten spaces instead.

Looks great otherwise. Let me know if you'd prefer if I made the above changes.
Comment 33 Amod [:AMoz] 2012-09-08 11:35:51 PDT
Created attachment 659508 [details] [diff] [review]
patch !
Comment 34 Dão Gottwald [:dao] 2012-09-08 11:54:44 PDT
Comment on attachment 659508 [details] [diff] [review]
patch !

Thanks!
Comment 35 Amod [:AMoz] 2012-09-08 11:59:42 PDT
Finally it was done..Thanks a lot sir. Also do i need to use commit statement. Many of my colleagues told me to do so. I am not much aware of it. Also after this what stages this bug would go under ?
Comment 36 Dão Gottwald [:dao] 2012-09-08 12:19:57 PDT
See https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch#Committing_the_patch

However you don't need to do anything in this case. I'll land your patch in a bit.
Comment 38 Amod [:AMoz] 2012-09-08 13:16:09 PDT
thank you very much !!! Now Looking for another bug mentored by you.
Comment 39 :Ehsan Akhgari (busy, don't ask for review please) 2012-09-08 13:56:54 PDT
This patch caused a number of test failures (see this log as an example: <https://tbpl.mozilla.org/php/getParsedLog.php?id=15078676&tree=Mozilla-Inbound>) so I backed it out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d2e26084434
Comment 40 Dão Gottwald [:dao] 2012-09-08 14:01:30 PDT
The browser_bug521216.js change is wrong ('actual' is an array). Re-landed with that dropped:

https://hg.mozilla.org/integration/mozilla-inbound/rev/11afe59db504
Comment 41 Ryan VanderMeulen [:RyanVM] 2012-09-08 18:11:42 PDT
https://hg.mozilla.org/mozilla-central/rev/11afe59db504
Comment 42 Amod [:AMoz] 2012-09-08 22:34:26 PDT
Thanks !!!
Comment 43 :Ms2ger 2012-09-17 04:03:59 PDT
Comment on attachment 659508 [details] [diff] [review]
patch !

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

::: browser/base/content/browser.js
@@ +1047,5 @@
>        new nsBrowserAccess();
>  
>      // set default character set if provided
>      if ("arguments" in window && window.arguments.length > 1 && window.arguments[1]) {
> +      if (window.arguments[1].startsWith("charset=")) {

Why is this startsWith instead of contains?
Comment 44 Dão Gottwald [:dao] 2012-09-17 04:43:59 PDT
Because "charset=" isn't expected to be preceded by anything.

Note You need to log in before you can comment on or make changes to this bug.