What is the aMode parameter in CToken::Consume?

RESOLVED FIXED in Future

Status

()

P3
minor
RESOLVED FIXED
18 years ago
16 years ago

People

(Reporter: dbaron, Assigned: harishd)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

I ran into this while trying to work on mode detection stuff...

The |aMode| parameter in |CToken::Consume| is used inconsistently.  In some
subclasses of CToken (such as CStartToken, CEndToken), there is a comment saying:

@param   aMode -- 1=HTML; 0=text (or other ML)

However, in CTextToken::ConsumeUntil and CCommentToken::Consume, aMode is
treated as an |nsDTDMode| (which is an |enum| type).

In CAttributeToken, there is no |aMode| parameter in the implementation (it is
called |aRetainWhitespace| instead|, although it is called |aMode| in the header
file.  In most of the other subclasses in nsHTMLTokens, |aMode| is just left unused.

Should this be consistent?  (I would think so, since |Consume| is a virtual
method.)  If not, why is |aMode| a parameter in the definition of CToken and in
all of the classes where it is unused?  If so, how should we fix it?  In some
places it's used like a PRBool (in 2 different ways), and others it's used like
an nsDTDMode.

Any thoughts on what was intended here?
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
Target Milestone: --- → mozilla0.9

Comment 1

18 years ago
updated qa contact.
QA Contact: janc → bsharma
Reality check.  Moving out to 0.9.1.
Target Milestone: mozilla0.9 → mozilla0.9.1
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Target Milestone: mozilla0.9.2 → mozilla0.9.3
->harishd
Assignee: dbaron → harishd
Status: ASSIGNED → NEW
(Assignee)

Comment 4

18 years ago
aMode parameter has been replaced with aFlag ( may be it should be called aFlags
)and is being used consistently.

Marking fixed.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
It's still called |aMode| in the header and |aFlag| in the implementation. 
That's annoyingly inconsistent.  Reopening, although it's not as serious as the
original problem.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

18 years ago
ok ok sigh!!!!
Status: REOPENED → ASSIGNED
Target Milestone: mozilla0.9.3 → mozilla1.0.1

Updated

17 years ago
QA Contact: bsharma → moied
(Assignee)

Comment 7

17 years ago
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug, or if it blocks your work in some way
-- please attach your concern to the bug for reconsideration. 
Severity: normal → minor
Target Milestone: mozilla1.0.1 → Future
Created attachment 119782 [details] [diff] [review]
Changes aMode to aFlag
Attachment #119782 - Flags: superreview?(jst)
Attachment #119782 - Flags: review?(harishd)
Comment on attachment 119782 [details] [diff] [review]
Changes aMode to aFlag

r+sr=jst, go ahead and land this whenever anyone has a chance.
Attachment #119782 - Flags: superreview?(jst)
Attachment #119782 - Flags: superreview+
Attachment #119782 - Flags: review?(harishd)
Attachment #119782 - Flags: review+
timeless landed this earlier today.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.