Closed Bug 63188 Opened 24 years ago Closed 22 years ago

What is the aMode parameter in CToken::Consume?

Categories

(Core :: DOM: HTML Parser, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: dbaron, Assigned: harishd)

Details

Attachments

(1 file)

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
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
aMode parameter has been replaced with aFlag ( may be it should be called aFlags )and is being used consistently. Marking fixed.
Status: NEW → RESOLVED
Closed: 24 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 → ---
ok ok sigh!!!!
Status: REOPENED → ASSIGNED
Target Milestone: mozilla0.9.3 → mozilla1.0.1
QA Contact: bsharma → moied
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
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
Closed: 24 years ago22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: