Closed Bug 324865 Opened 14 years ago Closed 14 years ago

Move XMLHttpRequest to gklayout

Categories

(Core :: XML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: sicking, Assigned: peterv)

References

Details

(Keywords: fixed1.8.1)

Attachments

(5 files, 3 obsolete files)

We need to get XMLHttpRequest into gklayout to be able to use some GC helper utilities that live there. And we're never going to ship gecko without XMLHttpRequest anyway so there's not much point to keeping it in mozilla/extensions.

So we could either move it to mozilla/content/base, or to dom/src/base. Oppinions? Takers?
I vote for mozilla/content/base.
Assignee: xml → peterv
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
(In reply to comment #2)
> also note bug 315890

Sure, but that didn't help for DOMCI.
Attached patch Changes to existing files - v1 (obsolete) — Splinter Review
We hit an assert in nsDOMClassInfo::PostCreate (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsDOMClassInfo.cpp&rev=1.351#3285) because nsIDOMParser should really be named nsIDOMDOMParser.
Status: NEW → ASSIGNED
Attached patch Changes to existing files - v1.1 (obsolete) — Splinter Review
This one doesn't hit assertions in DOMCI.
Attachment #210888 - Attachment is obsolete: true
The interface isn't frozen so we could in theory rename it nsIDOMDOMParser
Attached file CVS moves - v1
Comment on attachment 211260 [details]
CVS moves - v1

Looking for moa on the new locations. I thought about moving them to content/xml, but that'd require a new content/xml subdirectory (doesn't really fit in content or document).
Attachment #211260 - Flags: review?(jst)
Alternativly, we could merge the xml/content/* and xml/document/* directories into just xml/*. But that might be a bigger change then this bug.
Assignee: peterv → xml
Status: ASSIGNED → NEW
reverting accidental owner change
Assignee: xml → peterv
Status: NEW → ASSIGNED
Comment on attachment 211260 [details]
CVS moves - v1

r+sr=jst
Attachment #211260 - Flags: superreview+
Attachment #211260 - Flags: review?(jst)
Attachment #211260 - Flags: review+
Depends on: 332903
Attached patch v1.1Splinter Review
Attachment #210889 - Attachment is obsolete: true
Attachment #211137 - Attachment is obsolete: true
Attachment #219137 - Flags: superreview?(jst)
Attachment #219137 - Flags: review?(jst)
Attachment #219138 - Flags: superreview?(jst)
Attachment #219138 - Flags: review?(jst)
Blocks: 198595
Comment on attachment 219137 [details] [diff] [review]
v1.1

r+sr=jst
Attachment #219137 - Flags: superreview?(jst)
Attachment #219137 - Flags: superreview+
Attachment #219137 - Flags: review?(jst)
Attachment #219137 - Flags: review+
Attachment #219138 - Flags: superreview?(jst)
Attachment #219138 - Flags: superreview+
Attachment #219138 - Flags: review?(jst)
Attachment #219138 - Flags: review+
Finally done, sorry about the delays.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 335687
Blocks: 336821
Blocks: 336244
No longer blocks: 336821
Comment on attachment 219137 [details] [diff] [review]
v1.1

So I'd like to merge this to the 1.8 branch (see bug 336791), but I'm going to request approval for this one separately because it's a large patch, and I'd rather not have it be part of my diff in bug 336791.
Attachment #219137 - Flags: approval-branch-1.8.1?(peterv)
Attachment #219137 - Flags: approval-branch-1.8.1?(peterv)
Here's an actual merged patch.  I'll also attach a diff showing what changed after doing the cvs adds and removes.

I haven't done any runtime testing of this yet since I built with a bad configuration.  I'll be able to test it in a few hours, but hoping to get approval before you're asleep, perhaps.
Attachment #224937 - Flags: approval-branch-1.8.1?(peterv)
Here's the interdiff showing what actually changed other than the moves.  Note that I had to merge in some constructor bits from bug 314931 as well.
Patch seems fine; Google Maps and GMail both work.
I landed this on MOZILLA_1_8_BRANCH on the basis of a discussion I had a few weeks ago with jst; I couldn't find anyone else around to approve, and I needed it in to finish merging bug 336791.
Keywords: fixed1.8.1
(Some after-the-fact review of the merging would still be good.)
Attachment #224937 - Flags: approval-branch-1.8.1?(peterv) → approval-branch-1.8.1+
Ah, I missed the dependency on bug 335687; I'm not sure how easy it is to make that change without knowing the tools...
You also didn't pick up the allmakefiles.sh change and some of the packaging changes from bug 336244. Nothing major, but still nice to have.
> Ah, I missed the dependency on bug 335687; I'm not sure how easy it is to make
> that change without knowing the tools... 

I had Mano check in Camino project changes that should unbust maya on the 18branch next cycle.  (It's basically a 30sec fix, provided you're still running 10.3.9 and  Xcode 1.5--which is basically just me these days....)
You need to log in before you can comment on or make changes to this bug.