If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsIResProtocolHandler.idl shouldn't #include nsAString.h

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Networking
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Mook (songbird dead account), Assigned: Mook (songbird dead account))

Tracking

Trunk
mozilla1.9.2a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

837 bytes, patch
Benjamin Smedberg
: review+
Benjamin Smedberg
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Created attachment 365329 [details] [diff] [review]
use nsStringGlue.h

See summary; the use of nsAString.h makes it impossible to use from outside of libxul.  Simply switching to nsStringGlue.h makes it usable.

(Yes, it's not frozen.  It's also the only way to add something that doesn't refer to things chrome.manifest can handle - e.g. paths not known ahead of time.)
Attachment #365329 - Flags: review?(benjamin)
does it have to include this at all?

Updated

9 years ago
Attachment #365329 - Flags: review?(benjamin) → review-

Comment 2

9 years ago
Comment on attachment 365329 [details] [diff] [review]
use nsStringGlue.h

Just remove the include altogether, it's superfluous.

Note that this is not the only way to set a resource:// mapping. You can do so by creating a directory service key for "resource:mappingname"
(Assignee)

Comment 3

9 years ago
Created attachment 365443 [details] [diff] [review]
remove it

Okay, this removes it; I was originally thinking something else might have relied on that #include for the string definitions.  It looks like the only things that use this file from C++ are

/netwerk/protocol/res/src/nsResProtocolHandler.cpp
which has it from nsResProtocolHandler.h -> nsInterfaceHashtable.h -> nsHashKeys.h -> nsStringGlue.h

/netwerk/test/TestRes.cpp which isn't listed in its Makefile (and can't possibly work anyway, it wants nsIEventQueueService.h)

/chrome/src/nsChromeRegistry.cpp which manually #includes nsString.h

Thanks for the fast review, by the way :)
Assignee: nobody → mook
Attachment #365329 - Attachment is obsolete: true
Attachment #365443 - Flags: superreview?(benjamin)
Attachment #365443 - Flags: review?(benjamin)

Updated

9 years ago
Attachment #365443 - Flags: superreview?(benjamin)
Attachment #365443 - Flags: superreview+
Attachment #365443 - Flags: review?(benjamin)
Attachment #365443 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
OS: Windows Vista → All
Hardware: x86 → All
http://hg.mozilla.org/mozilla-central/rev/7db7d5ab702a
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: nsIResProtocolHandler.idl #includes nsAString.h instead of nsStringGlue.h → nsIResProtocolHandler.idl shouldn't #include nsAString.h
Target Milestone: --- → mozilla1.9.2a1

Updated

8 years ago
Blocks: 357052
You need to log in before you can comment on or make changes to this bug.