Closed Bug 366774 Opened 18 years ago Closed 14 years ago

Remove unused preference bidi.characterset

Categories

(Core :: Layout: Text and Fonts, enhancement)

Product:
Core
Component:
Layout: Text and Fonts
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla6

People

(Reporter: fahlmanc_ca, Assigned: ewong)

Details

Attachments

(1 file, 9 obsolete files)

Remove unused preference bidi.characterset(v4)
7.99 KB, patch
ewong
: review+
Details | Diff | Splinter Review
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.
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
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #520496 - Flags: review? → review?(benjamin)
Attachment #520497 - Flags: review?(bzbarsky)
Attachment #520498 - Flags: review?(benjamin)
Attachment #520499 - Flags: review? → review?(mrbkap)
Attachment #520500 - Flags: review?(smontagu)
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 #520500 - Attachment is obsolete: true
Attachment #520502 - Flags: review?(smontagu)
Attachment #520500 - Flags: review?(smontagu)
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+
Attachment #520499 - Flags: review?(mrbkap) → review+
Attachment #520498 - Attachment is obsolete: true
Attachment #520506 - Flags: 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
Keywords: checkin-needed
Turned the whole tree red, I think because patch 5 has a missing close paren.
Ah right: + (IBMBIDI_SUPPORTMODE_MOZILLA<<12) should be: + (IBMBIDI_SUPPORTMODE_MOZILLA<<12))
Fixed missing parenthesis.
Attachment #520502 - Attachment is obsolete: true
Attachment #528059 - Flags: review+
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"
Attachment #528093 - Attachment is obsolete: true
Attachment #529345 - Flags: review+
So which patch needs to be checked in? It'd be nice to mark obsolete patches as obsolete....
Anyone?
Attachment #528059 - Attachment is obsolete: true
Attachment #520506 - Attachment is obsolete: true
Attachment #520497 - Attachment is obsolete: true
Attachment #520496 - Attachment is obsolete: true
Keywords: checkin-needed
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/95dad5bda710
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 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.

Attachment

General

Creator:
Created:
Updated:
Size: