The default bug view has changed. See this FAQ.

Split charset source constants out of nsIParser.h

RESOLVED FIXED in mozilla14

Status

()

Core
HTML: Parser
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Assignee)

Description

5 years ago
There should be parser/nsCharsetSource.h for the kCharset* constants that are now in nsIParser.h to make fewer places include nsIParser.h.

Note that in order to avoid burning comm-central, the first patch should make nsIParser.h include the new nsCharsetSource.h. Then comm-central can move to including nsCharsetSource.h directly and then nsIParser.h can stop including nsCharsetSource.h.
(Assignee)

Comment 1

5 years ago
Taking this myself in order to get a better idea of the remaining real nsIParser deps.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Whiteboard: [mentor=hsivonen][lang=C++]
(Assignee)

Comment 2

5 years ago
Created attachment 607957 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes
Attachment #607957 - Flags: review?(bugs)
(Assignee)

Comment 3

5 years ago
Created attachment 607958 [details] [diff] [review]
Part 3: Remove comm-central scaffolding

Part 2 will be a comm-central patch.
Attachment #607958 - Flags: review?(bugs)
(Assignee)

Comment 4

5 years ago
Created attachment 607959 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, without accidental file removal
Attachment #607957 - Attachment is obsolete: true
Attachment #607957 - Flags: review?(bugs)
(Assignee)

Updated

5 years ago
Attachment #607959 - Flags: review?(bugs)
Comment on attachment 607959 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, without accidental file removal

Review of attachment 607959 [details] [diff] [review]:
-----------------------------------------------------------------

::: parser/Makefile.in
@@ +46,5 @@
>  DIRS            = expat xml htmlparser html
>  
> +EXPORTS		= \
> +		nsCharsetSource.h \
> +		$(NULL)

Just

EXPORTS = \
  nsCharsetSource.h \
  $(NULL)

please, no tabs
(Assignee)

Comment 6

5 years ago
Created attachment 607965 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, for real the right patch
Attachment #607959 - Attachment is obsolete: true
Attachment #607959 - Flags: review?(bugs)
Attachment #607965 - Flags: review?(bugs)
(Assignee)

Comment 7

5 years ago
Created attachment 607972 [details] [diff] [review]
Part 2: Remove nsIParser.h from comm-central
(Assignee)

Comment 8

5 years ago
Created attachment 607974 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, Ms2ger's comments addressed
Attachment #607965 - Attachment is obsolete: true
Attachment #607965 - Flags: review?(bugs)
Attachment #607974 - Flags: review?(bugs)

Updated

5 years ago
Attachment #607974 - Flags: review?(bugs) → review+

Updated

5 years ago
Attachment #607958 - Flags: review?(bugs) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 608302 [details] [diff] [review]
Part 2: Remove nsIParser.h from comm-central
Attachment #607972 - Attachment is obsolete: true
Attachment #608302 - Flags: review?(mbanner)
(Assignee)

Updated

5 years ago
Attachment #608302 - Attachment is obsolete: true
Attachment #608302 - Flags: review?(mbanner)
(Assignee)

Comment 10

5 years ago
Created attachment 608303 [details] [diff] [review]
Part 2: Remove nsIParser.h and nsIHTMLContentSink.h from comm-central

Let's remove the nsIHTMLContentSink.h #include while at it.
Attachment #608303 - Flags: review?(mbanner)
(Assignee)

Updated

5 years ago
Attachment #608303 - Attachment is patch: true
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/323deae312d2
Whiteboard: [please leave open after inbound merge; parts 2 and 3 need to land still]
https://hg.mozilla.org/mozilla-central/rev/323deae312d2
Comment on attachment 608303 [details] [diff] [review]
Part 2: Remove nsIParser.h and nsIHTMLContentSink.h from comm-central

I've not tested it, but this looks fine.
Attachment #608303 - Flags: review?(mbanner) → review+
(Assignee)

Comment 14

5 years ago
Thanks for the reviews.

Landed parts 2 and 3:
http://hg.mozilla.org/comm-central/rev/d5bc8c4eeb2f
http://hg.mozilla.org/integration/mozilla-inbound/rev/c2a5e0dba377
Whiteboard: [please leave open after inbound merge; parts 2 and 3 need to land still]
https://hg.mozilla.org/mozilla-central/rev/c2a5e0dba377
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.