Closed
Bug 729048
Opened 12 years ago
Closed 12 years ago
Move nsParserService::CheckQName to nsContentUtils
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: hsivonen, Assigned: dwarfcrank)
References
(Blocks 1 open bug)
Details
(Whiteboard: [mentor=hsivonen][lang=C++])
Attachments
(5 files, 4 obsolete files)
1.40 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
1.42 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
Part 4: Replace calls to nsParserService::CheckQName with nsContentUtils::CheckQName in txXMLUtils.h
1.16 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
3.53 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
5.24 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
To make it possible to get rid of nsIParserService and to make code less COMtaminated, move nsParserService::CheckQName to nsContentUtils as a static.
Reporter | ||
Updated•12 years ago
|
Whiteboard: [mentor=hsivonen][lang=C++]
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #608689 -
Flags: review?(hsivonen)
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #608690 -
Flags: review?(hsivonen)
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #608691 -
Flags: review?(hsivonen)
Assignee | ||
Updated•12 years ago
|
Attachment #608691 -
Attachment is patch: true
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #608692 -
Flags: review?(hsivonen)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #608693 -
Flags: review?(hsivonen)
Assignee | ||
Comment 6•12 years ago
|
||
Whoops, accidentally inserted tabs instead of spaces, apologies!
Attachment #608689 -
Attachment is obsolete: true
Attachment #608704 -
Flags: review?(hsivonen)
Attachment #608689 -
Flags: review?(hsivonen)
Assignee | ||
Comment 7•12 years ago
|
||
Forgot to remove commented-out parts.
Attachment #608693 -
Attachment is obsolete: true
Attachment #608693 -
Flags: review?(hsivonen)
Assignee | ||
Updated•12 years ago
|
Attachment #608706 -
Attachment is patch: true
Attachment #608706 -
Flags: review?(hsivonen)
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 608704 [details] [diff] [review] Part 1: Move nsParserService::CheckQName to nsContentUtils and change calls within nsContentUtils to use nsContentUtils::CheckQName instead. ># HG changeset patch ># Parent c20ec27eb0e8ec666a987a6f71a3902257f8128d ># User Matias Juntunen <matias.juntunen@gmail.com> >Bug 729048 - Move nsParserService::CheckQName to nsContentUtils (part 1): Move nsParserService::CheckQName to nsContentUtils and change calls within nsContentUtils to use nsContentUtils::CheckQName instead. >+ const PRUnichar** aColon = NULL); Please use nsnull instead of NULL. >+ const char* colon = NULL; Also here. >+ if (result == 0) { if (!result) { r=hsivonen if those nits are fixed.
Attachment #608704 -
Flags: review?(hsivonen) → review+
Reporter | ||
Updated•12 years ago
|
Attachment #608690 -
Flags: review?(hsivonen) → review+
Reporter | ||
Updated•12 years ago
|
Attachment #608691 -
Flags: review?(hsivonen) → review+
Reporter | ||
Updated•12 years ago
|
Attachment #608692 -
Flags: review?(hsivonen) → review+
Reporter | ||
Updated•12 years ago
|
Attachment #608706 -
Flags: review?(hsivonen) → review+
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → matias.juntunen
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•12 years ago
|
||
Fixed the issues that were pointed out.
Attachment #608704 -
Attachment is obsolete: true
Attachment #609404 -
Flags: review?(hsivonen)
Comment 10•12 years ago
|
||
Comment on attachment 609404 [details] [diff] [review] Part 1: Move nsParserService::CheckQName to nsContentUtils and change calls within nsContentUtils to use nsContentUtils::CheckQName instead. Review of attachment 609404 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsContentUtils.cpp @@ +2399,5 @@ > + reinterpret_cast<const char*>(end), > + aNamespaceAware, &colon); > + > + if (!result) { > + if(aColon) { One more nit: 'if ('
Assignee | ||
Comment 11•12 years ago
|
||
There, now it should be fine. Sorry for wasting your time with these formatting mistakes!
Attachment #609404 -
Attachment is obsolete: true
Attachment #609404 -
Flags: review?(hsivonen)
Reporter | ||
Comment 12•12 years ago
|
||
Comment on attachment 609409 [details] [diff] [review] Part 1: Move nsParserService::CheckQName to nsContentUtils and change calls within nsContentUtils to use nsContentUtils::CheckQName instead. Are you familiar with the landing procedure?
Attachment #609409 -
Flags: review+
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #12) > Comment on attachment 609409 [details] [diff] [review] > Part 1: Move nsParserService::CheckQName to nsContentUtils and change calls > within nsContentUtils to use nsContentUtils::CheckQName instead. > > Are you familiar with the landing procedure? Somewhat, I have submitted one patch before. Basically, after the patches have been reviewed and passed the review, one has to ask for someone with the right privileges to add the checkin-needed keyword (after making sure the patches don't break the current tree). After that one waits for it to be eventually pushed into mozilla-incoming. Right?
Reporter | ||
Comment 14•12 years ago
|
||
(In reply to Matias Juntunen from comment #13) > Basically, after the patches have been reviewed and passed the review, one > has to ask for someone with the right privileges to add the checkin-needed > keyword (after making sure the patches don't break the current tree). After > that one waits for it to be eventually pushed into mozilla-incoming. Right? Not quite. You don't need to ask someone else to add "checkin-needed". The patch author typically adds the keyword.
Assignee | ||
Comment 15•12 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #14) > Not quite. You don't need to ask someone else to add "checkin-needed". The > patch author typically adds the keyword. Ah, okay. It says here: https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch#Committing_the_patch that I should ask someone to add the keyword.
Keywords: checkin-needed
Comment 16•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/11ef97686043 https://hg.mozilla.org/integration/mozilla-inbound/rev/7af2fcef270f https://hg.mozilla.org/integration/mozilla-inbound/rev/c2d8ac6bb09c https://hg.mozilla.org/integration/mozilla-inbound/rev/db2978f38798 https://hg.mozilla.org/integration/mozilla-inbound/rev/4bbfb954c0dd
Reporter | ||
Comment 17•12 years ago
|
||
(In reply to Matias Juntunen from comment #15) > It says here: > https://developer.mozilla.org/En/Developer_Guide/ > How_to_Submit_a_Patch#Committing_the_patch that I should ask someone to add > the keyword. Oh OK. I fixed the wiki. :-) Thank you for fixing this!
Flags: in-testsuite? → in-testsuite-
Comment 18•12 years ago
|
||
Thank you for the patch - hopefully see you on IRC soon! :-) https://hg.mozilla.org/mozilla-central/rev/11ef97686043 https://hg.mozilla.org/mozilla-central/rev/7af2fcef270f https://hg.mozilla.org/mozilla-central/rev/c2d8ac6bb09c https://hg.mozilla.org/mozilla-central/rev/db2978f38798 https://hg.mozilla.org/mozilla-central/rev/4bbfb954c0dd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•