Last Comment Bug 366774 - Remove unused preference bidi.characterset
: Remove unused preference bidi.characterset
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla6
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-11 22:23 PST by crf
Modified: 2011-07-29 04:14 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Removed unused preference bidi.characterset (p1) (v1) (1.19 KB, patch)
2011-03-19 23:03 PDT, Edmund Wong (:ewong)
smontagu: review+
Details | Diff | Review
Removed unused preference bidi.characterset (p2) (v1) (2.31 KB, patch)
2011-03-19 23:05 PDT, Edmund Wong (:ewong)
smontagu: review+
Details | Diff | Review
Removed unused preference bidi.characterset (p3) (v1) (996 bytes, patch)
2011-03-19 23:05 PDT, Edmund Wong (:ewong)
smontagu: review+
Details | Diff | Review
Removed unused preference bidi.characterset (p4) (v1) (1.39 KB, patch)
2011-03-19 23:06 PDT, Edmund Wong (:ewong)
mrbkap: review-
Details | Diff | Review
Removed unused preference bidi.characterset (p5) (v1) (1.69 KB, patch)
2011-03-19 23:08 PDT, Edmund Wong (:ewong)
no flags Details | Diff | Review
Removed unused preference bidi.characterset (p5) (v2) (3.45 KB, patch)
2011-03-20 00:37 PDT, Edmund Wong (:ewong)
smontagu: review+
Details | Diff | Review
Removed unused preference bidi.characterset (p3) (v2) (1.04 KB, patch)
2011-03-20 03:03 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Review
Remove unused preference bidi.characterset p5 (v3) (3.45 KB, patch)
2011-04-24 21:08 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Review
Removed the unused preference bidi.characterset references. (7.97 KB, patch)
2011-04-25 07:38 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Review
Remove unused preference bidi.characterset(v4) (7.99 KB, patch)
2011-05-01 06:21 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Review

Description crf 2007-01-11 22:23:47 PST
User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-GB; rv:1.8.0.9) Gecko/20061211 SeaMonkey/1.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-GB; rv:1.8.0.9) Gecko/20061211 SeaMonkey/1.0.7

In about:config there is a setting bidi.characterset , but other settings use within them the abbreviated term charset. For consistency, bidi.characterset could be renamed to bidi.charset.





Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 1 Simon Montagu :smontagu 2007-01-13 20:46:16 PST
The setting isn't used and probably never will be (see bug 79676 comment 24, third paragraph from the end). Rather than renaming it, it should just be removed.
Comment 2 Tony Mechelynck [:tonymec] 2008-04-29 23:09:07 PDT
No comment in over a year.
Comment 3 neil@parkwaycc.co.uk 2008-05-01 08:41:53 PDT
(In reply to comment #1)
> The setting isn't used and probably never will be
I just traced this. There is an attribute bidiCharacterSet on nsIMarkupDocumentViewer.idl (implemented by nsDocumentViewer.cpp) which calls through to nsPresContext.cpp which reads the default from preferences.

Nobody actually uses the attribute ;-)
Comment 4 Tony Mechelynck [:tonymec] 2010-12-11 00:17:30 PST
Editing the Subject to reflect what was said to comment #1 and #3, and confirming.
Comment 5 Edmund Wong (:ewong) 2011-03-19 23:03:54 PDT
Created attachment 520496 [details] [diff] [review]
Removed unused preference bidi.characterset (p1) (v1)
Comment 6 Edmund Wong (:ewong) 2011-03-19 23:05:14 PDT
Created attachment 520497 [details] [diff] [review]
Removed unused preference bidi.characterset (p2) (v1)
Comment 7 Edmund Wong (:ewong) 2011-03-19 23:05:54 PDT
Created attachment 520498 [details] [diff] [review]
Removed unused preference bidi.characterset (p3) (v1)
Comment 8 Edmund Wong (:ewong) 2011-03-19 23:06:34 PDT
Created attachment 520499 [details] [diff] [review]
Removed unused preference bidi.characterset (p4) (v1)
Comment 9 Edmund Wong (:ewong) 2011-03-19 23:08:55 PDT
Created attachment 520500 [details] [diff] [review]
Removed unused preference bidi.characterset (p5) (v1)
Comment 10 Simon Montagu :smontagu 2011-03-19 23:41:19 PDT
Comment on attachment 520500 [details] [diff] [review]
Removed unused preference bidi.characterset (p5) (v1)

> //  ------------------
> //  Charset Mode
> //  ------------------

These comment lines should be removed, and also the following #defines:
 IBMBIDI_CHARSET_STR
 IBMBIDI_CHARSET
 GET_BIDI_OPTION_CHARACTERSET
 SET_BIDI_OPTION_CHARACTERSET

>+#define IBMBIDI_NULL      0 // placeholder for future use.

This isn't necessary.
Comment 11 Edmund Wong (:ewong) 2011-03-20 00:37:52 PDT
Created attachment 520502 [details] [diff] [review]
Removed unused preference bidi.characterset (p5) (v2)
Comment 12 Simon Montagu :smontagu 2011-03-20 00:57:44 PDT
Comment on attachment 520496 [details] [diff] [review]
Removed unused preference bidi.characterset (p1) (v1)

If nobody objects, I think my review should be good for the other patches here too.
Comment 13 Simon Montagu :smontagu 2011-03-20 00:59:35 PDT
Comment on attachment 520498 [details] [diff] [review]
Removed unused preference bidi.characterset (p3) (v1)

> // ------------------
> //  Charset Mode
> // ------------------

Delete these lines here too. r=me with that
Comment 14 Edmund Wong (:ewong) 2011-03-20 03:03:29 PDT
Created attachment 520506 [details] [diff] [review]
Removed unused preference bidi.characterset (p3) (v2)
Comment 15 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-03-21 09:44:34 PDT
Comment on attachment 520499 [details] [diff] [review]
Removed unused preference bidi.characterset (p4) (v1)

This isn't really necessary. This is just a test that's using the pref as part of some data.
Comment 16 Blake Kaplan (:mrbkap) (please use needinfo!) 2011-03-21 09:45:23 PDT
By "pref" I meant "pref name".
Comment 18 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-24 11:21:26 PDT
Turned the whole tree red, I think because patch 5 has a missing close paren.
Comment 20 Philip Chee 2011-04-24 11:35:16 PDT
Ah right:
+         (IBMBIDI_SUPPORTMODE_MOZILLA<<12)
should be:
+         (IBMBIDI_SUPPORTMODE_MOZILLA<<12))
Comment 21 Edmund Wong (:ewong) 2011-04-24 21:08:46 PDT
Created attachment 528059 [details] [diff] [review]
Remove unused preference bidi.characterset p5 (v3)

Fixed missing parenthesis.
Comment 22 :Ms2ger 2011-04-25 00:33:55 PDT
Edmund: if you don't mind, it'd be nice to have commit messages that more clearly explain what each patch does. Also, it seems like part 1 wouldn't compile without part 2, so these should be merged, and you need to update the uuid for nsIMarkupDocumentViewer and nsIMarkupDocumentViewer_MOZILLA_2_0_BRANCH.
Comment 23 Philip Chee 2011-04-25 03:58:52 PDT
> nsIMarkupDocumentViewer_MOZILLA_2_0_BRANCH.

Ixnay on this as there is an existing bug in progress to remove all *MOZILLA_2_0_BRANCH interfaces from mozilla-central.
Comment 24 Edmund Wong (:ewong) 2011-04-25 07:38:05 PDT
Created attachment 528093 [details] [diff] [review]
Removed the unused preference bidi.characterset references.

This patch merges all the aforementioned patches into one patch with the following changes:

Changed the UUID for nsIMarkupDocumentViewer and
nsIMarkupDocumentViewer_MOZILLA_2_0_BRANCH.
Comment 25 Philip Chee 2011-04-25 10:06:08 PDT
You *did* push to tryserver this time didn't you?
Comment 26 Edmund Wong (:ewong) 2011-04-29 18:51:54 PDT
(In reply to comment #25)
> You *did* push to tryserver this time didn't you?

Pushed to try with a green tree.
Comment 27 :Ms2ger 2011-05-01 01:33:25 PDT
Though the commit message should include "r=smontagu"
Comment 28 Edmund Wong (:ewong) 2011-05-01 06:21:39 PDT
Created attachment 529345 [details] [diff] [review]
Remove unused preference bidi.characterset(v4)
Comment 29 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-03 09:27:07 PDT
So which patch needs to be checked in?  It'd be nice to mark obsolete patches as obsolete....
Comment 30 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-05 13:57:27 PDT
Anyone?
Comment 31 :Ms2ger 2011-05-10 10:05:36 PDT
Sure.
Comment 33 Simona B [:simonab] 2011-07-29 04:14:57 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue using Windows XP, Windows 7, Ubuntu and Mac OS X - bidi.characterset preference is removed about:config.

Setting resolution to VERIFIED FIXED.

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