Closed Bug 366774 Opened 14 years ago Closed 9 years ago
Remove unused preference bidi
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-GB; rv:126.96.36.199) Gecko/20061211 SeaMonkey/1.0.7 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-GB; rv:188.8.131.52) 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.
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.
OS: Windows 98 → All
Hardware: PC → All
No comment in over a year.
Assignee: general → nobody
Component: General → Preferences
QA Contact: general → prefs
Version: unspecified → Trunk
(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 ;-)
Editing the Subject to reflect what was said to comment #1 and #3, and confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Rename the setting bidi.characterset to bidi.charset to be consistent → Remove unused preference bidi.characterset
Attachment #520496 - Flags: review? → review?(benjamin)
Attachment #520499 - Flags: review? → review?(mrbkap)
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.
Attachment #520502 - Flags: review?(smontagu) → review+
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.
Attachment #520496 - Flags: review?(benjamin) → review+
Attachment #520497 - Flags: review?(bzbarsky) → review+
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
Attachment #520498 - Flags: review?(benjamin) → review+
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.
Attachment #520499 - Flags: review+ → review-
By "pref" I meant "pref name".
Attachment #520499 - Attachment is obsolete: true
Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/582c92878440d956e227341c66603de34d2e3b8e http://hg.mozilla.org/mozilla-central/rev/baf7456adc9854ea331f9defc096beb1d2990145 http://hg.mozilla.org/mozilla-central/rev/3f7277e15d8df0907d14d50f194cc0cef17f7da4 http://hg.mozilla.org/mozilla-central/rev/96bfa1480f5f9332650034b503cc34240c6579fd
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.2a1
Turned the whole tree red, I think because patch 5 has a missing close paren.
Backed out: https://hg.mozilla.org/mozilla-central/rev/9bc24665869d https://hg.mozilla.org/mozilla-central/rev/e49949eeaef3 https://hg.mozilla.org/mozilla-central/rev/2928c63ffb5b https://hg.mozilla.org/mozilla-central/rev/8db1df84708c https://hg.mozilla.org/mozilla-central/rev/cb191997bd5e https://hg.mozilla.org/mozilla-central/rev/54b9f7d8db85 https://hg.mozilla.org/mozilla-central/rev/cca1fece061f https://hg.mozilla.org/mozilla-central/rev/8302f5191d92
Assignee: ewong → nobody
Status: RESOLVED → REOPENED
Component: Preferences → Layout: Text
Product: SeaMonkey → Core
QA Contact: preferences → layout.fonts-and-text
Resolution: FIXED → ---
Target Milestone: seamonkey2.2a1 → ---
Ah right: + (IBMBIDI_SUPPORTMODE_MOZILLA<<12) should be: + (IBMBIDI_SUPPORTMODE_MOZILLA<<12))
Fixed missing parenthesis.
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.
> 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.
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.
Attachment #528093 - Flags: review+
You *did* push to tryserver this time didn't you?
Status: REOPENED → ASSIGNED
(In reply to comment #25) > You *did* push to tryserver this time didn't you? Pushed to try with a green tree.
Though the commit message should include "r=smontagu"
So which patch needs to be checked in? It'd be nice to mark obsolete patches as obsolete....
Attachment #528059 - Attachment is obsolete: true
Attachment #520506 - Attachment is obsolete: true
Attachment #520497 - Attachment is obsolete: true
Attachment #520496 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.