remove case-insensitive compares from nsStringArray

RESOLVED FIXED in mozilla0.9.6

Status

()

Core
XPCOM
P2
normal
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: Alec Flett, Assigned: Alec Flett)

Tracking

Trunk
mozilla0.9.6
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
As a part of bug 100214:
We are removing case-insensitive comparisons on PRUnichar strings from XPCOM.
This means that nsStringArray needs to get rid of the three methods that require
this capability:
IndexOfIgnoreCase()
RemoveStringIgnoreCase()
SortIgnoreCase()

Fortunately, the only consumer of any of these routines, namely
IndexOfIgnoreCase, is nsPresShell, for case-insensitively identifying alternate
style sheets. I looked at the w3c spec, and it gives no indication about if case
matters in the name of an alternate style sheet, so I would like to propose a
simple solution whereby we simply make alternate-stylesheet selection
case-sensitive.
The routines in question are:
GetActiveAlternateStyleSheet()
SelectAlternateStyleSheet()
ListAlternateStyleSheets()

Of these, only SelectAlternateStyleSheet and ListAlternateStyleSheet are
affected by this change, and both of these routines are ONLY used by the viewer
test application.

The patch I'm about to attach removes the aforementioned routines and changes
behavior such that:
- ListAlternateStyleSheets() no longer attempts uses case-insensitive compares
to prune the list
- SelectAlternateStyleSheet() uses IndexOf() rather than IndexOfIgnoreCase()

This way we have symmetry with respect to the list and selection of style sheets.
(Assignee)

Comment 1

16 years ago
Created attachment 52988 [details] [diff] [review]
fix up the nsStringArray API
(Assignee)

Comment 2

16 years ago
If everyone is ok with this, can I get an r=jag and an sr=attinasi?
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.6
(Assignee)

Updated

16 years ago
Blocks: 100214
(Assignee)

Updated

16 years ago
No longer blocks: 100214
(Assignee)

Updated

16 years ago
Blocks: 100214

Comment 3

16 years ago
There is the ability to select alternate style sheets int eh Mozill aUI - are
you sure it is not using those methods? Regardless, I doubt they need to be
case-insensitive.

Comment 4

16 years ago
Comment on attachment 52988 [details] [diff] [review]
fix up the nsStringArray API

sr=attinasi - (I'm assuming that you tested to be sure that the alternate stylesheet UI in Mozilla still works)
Attachment #52988 - Flags: superreview+

Comment 5

16 years ago
Comment on attachment 52988 [details] [diff] [review]
fix up the nsStringArray API

r=jag
Attachment #52988 - Flags: review+
(Assignee)

Comment 6

16 years ago
actually the alternate style sheet ui in the browser is not hooked up to
anything.. it will always say "None", believe it or not.

the only place it's used is in viewer....thanks folks.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 7

16 years ago
Use http://www.w3.org/Style/CSS/ as a testcase for alternate stylesheets.

Comment 8

16 years ago
Alternate stylesheet UI works fine for me (20011003) and CVS build from 10-10

Alec, please recheck using the URL jag posted you should get a bunch of
alternate sheets, like 10 or 11.
(Assignee)

Comment 9

16 years ago
hey, you're right!
good news is that they still show up and work in today's build :)
You need to log in before you can comment on or make changes to this bug.