Last Comment Bug 737417 - Split charset source constants out of nsIParser.h
: Split charset source constants out of nsIParser.h
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-20 07:45 PDT by Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
Modified: 2012-03-25 06:12 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Split the constants out and remove useless nsIParser.h includes (29.23 KB, patch)
2012-03-21 08:05 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
no flags Details | Diff | Splinter Review
Part 3: Remove comm-central scaffolding (941 bytes, patch)
2012-03-21 08:06 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
bugs: review+
Details | Diff | Splinter Review
Part 1: Split the constants out and remove useless nsIParser.h includes, without accidental file removal (28.87 KB, patch)
2012-03-21 08:09 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
no flags Details | Diff | Splinter Review
Part 1: Split the constants out and remove useless nsIParser.h includes, for real the right patch (30.27 KB, patch)
2012-03-21 08:21 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
no flags Details | Diff | Splinter Review
Part 2: Remove nsIParser.h from comm-central (1.80 KB, patch)
2012-03-21 08:29 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
no flags Details | Diff | Splinter Review
Part 1: Split the constants out and remove useless nsIParser.h includes, Ms2ger's comments addressed (30.27 KB, patch)
2012-03-21 08:32 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
bugs: review+
Details | Diff | Splinter Review
Part 2: Remove nsIParser.h from comm-central (2.46 KB, patch)
2012-03-22 04:48 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
no flags Details | Diff | Splinter Review
Part 2: Remove nsIParser.h and nsIHTMLContentSink.h from comm-central (2.49 KB, patch)
2012-03-22 04:52 PDT, Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03)
standard8: review+
Details | Diff | Splinter Review

Description Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-20 07:45:23 PDT
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.
Comment 1 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 06:23:49 PDT
Taking this myself in order to get a better idea of the remaining real nsIParser deps.
Comment 2 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:05:55 PDT
Created attachment 607957 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes
Comment 3 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:06:40 PDT
Created attachment 607958 [details] [diff] [review]
Part 3: Remove comm-central scaffolding

Part 2 will be a comm-central patch.
Comment 4 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:09:06 PDT
Created attachment 607959 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, without accidental file removal
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2012-03-21 08:14:47 PDT
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
Comment 6 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:21:12 PDT
Created attachment 607965 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, for real the right patch
Comment 7 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:29:10 PDT
Created attachment 607972 [details] [diff] [review]
Part 2: Remove nsIParser.h from comm-central
Comment 8 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-21 08:32:10 PDT
Created attachment 607974 [details] [diff] [review]
Part 1: Split the constants out and remove useless nsIParser.h includes, Ms2ger's comments addressed
Comment 9 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-22 04:48:04 PDT
Created attachment 608302 [details] [diff] [review]
Part 2: Remove nsIParser.h from comm-central
Comment 10 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-22 04:52:33 PDT
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.
Comment 11 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-22 07:46:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/323deae312d2
Comment 12 Marco Bonardo [::mak] 2012-03-22 18:12:15 PDT
https://hg.mozilla.org/mozilla-central/rev/323deae312d2
Comment 13 Mark Banner (:standard8) 2012-03-23 05:25:01 PDT
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.
Comment 14 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-10-03) 2012-03-24 11:56:59 PDT
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
Comment 15 Ed Morley [:emorley] 2012-03-25 06:12:21 PDT
https://hg.mozilla.org/mozilla-central/rev/c2a5e0dba377

Note You need to log in before you can comment on or make changes to this bug.