Closed Bug 475898 Opened 15 years ago Closed 15 years ago

jsdIDebuggerService.idl changed without changing uuid

Categories

(Other Applications Graveyard :: Venkman JS Debugger, defect, P1)

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: pjemen, Assigned: timeless)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 XPCOMViewer/1.0a1
Build Identifier: Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081210 Thunderbird/3.0b1 

new method:
	void DumpHeap(in string fileName);


Reproducible: Always

Actual Results:  
old uuid

Expected Results:  
new uuid
Flags: wanted-thunderbird3?
Assignee: nobody → rginda
Component: General → JavaScript Debugger
Flags: wanted-thunderbird3?
Product: Thunderbird → Other Applications
QA Contact: general → venkman
So erm, this happened sometime in 2007. A few days ago, a new change was checked in, and all the UUIDs were updated. I'm fairly sure this is now unfixable - I mean, there are already (many) released binaries in the wild which have the same old UUID for this changed version of the interface. The only thing I can think of is changing it on branch, but I'm not sure what the merits of that are at this point.

Igor, opinions?
Status: UNCONFIRMED → NEW
Depends on: 378261
Ever confirmed: true
I am not the right person to ask that question as I I only recently has learned about the need to change ids when changing interfaces.
I do a binnary extension.
I want to support with one dll gecko 1.8 and 1.9.
For extension developer it is important to distinguish unfrozen interfaces. 

I want it in Thunderbird b2 if it is possible. 
I think it is gecko 1.9.1.

The best way is put it into all affected branches. Next build from that branch will fix this bug. 

Is it problem?

At least ,if it is possible yet give it into 1.9.1, 1.9.2 and into trunk.
This isn't only uuid interface bug I reported today.

I reported (open discussion) it to mozilla.dev.platform
Subject "Interface's uuid problem"

If you are not shre where to put patch ask there probably someone answers you.
As I said in comment #1, the UUID on trunk has already changed - changing it again won't help. A branch patch for 1.9.1 may be useful.

I don't believe there is a 1.9.2 branch - trunk is used for 1.9.2 development. If that is not the case, then that has been poorly communicated - I can't find mentions on planet.m.o or in the m.d.planning newsgroup, or on wiki.m.o .
Version: unspecified → 1.9.1 Branch
The problem with patching the stable branch is that that might break existing extensions written for it...
Flags: blocking1.9.1?
Version: 1.9.1 Branch → unspecified
"stable branch" == 1.9.0 here, since that's where the problem the reporter ran into actually exists...
(In reply to comment #7)
> "stable branch" == 1.9.0 here, since that's where the problem the reporter ran
> into actually exists...

Right, but fixing this branch is useless at this point, isn't it? I mean, there are already lots of users which will have a browser with an interface with a UUID that is the same, and somehow (!?) the add-on developer will already have to deal with this. Even if 1.9.0.6 is fixed, 1.9.0.1-5 will still be broken. Presumably there would be a hackaround for the developer for these builds, somehow, which could also be applied for any remaining releases off 1.9.0.

I will confess I am unfamiliar with binary add-ons, so I am not sure if there are reasons that make it worthwhile to still do this, impossible to hackaround, etc., so please feel free to enlighten me in this respect...
Given that this is in a prior stable release, I think it should be WONTFIX.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: blocking1.9.1? → blocking1.9.1-
Resolution: --- → WONTFIX
Firefox isn't the only thing we release based on Gecko.  There's no shipping 1.9.0-based tbird, for example...  Let's sort all of this out in .planning before closing bugs, ok?
Status: RESOLVED → REOPENED
Flags: blocking1.9.1- → blocking1.9.1?
Resolution: WONTFIX → ---
I meant .platform.
Attached patch Patch for 1.9.1 (obsolete) — Splinter Review
This is a patch against the 1.9.1 branch. Josh, if you are going to land your Unicode-izations on 1.9.1 (thus rev'ing the ID again), can you please let us know? I assumed you weren't, as it seems like a change that's a bit much to take at this point in the game for 1.9.1...
Assignee: rginda → gijskruitbosch+bugs
Status: REOPENED → ASSIGNED
Attachment #359539 - Flags: superreview?(bzbarsky)
Attachment #359539 - Flags: review?(timeless)
Attached patch proper style (obsolete) — Splinter Review
the patch that added this function wasn't given sr. and should never have been given sr. It violates a number of rules. interCaps method naming, adding methods at the end.

Since we're reving the interface anyway, i'd like to fix the name and location. We're breaking any consumer anyway, and the js ones can handle function case naming if they care (we did it for nsIRunnable years ago).
Assignee: gijskruitbosch+bugs → timeless
Attachment #359539 - Attachment is obsolete: true
Attachment #359641 - Flags: superreview?(bzbarsky)
Attachment #359641 - Flags: review?(gijskruitbosch+bugs)
Attachment #359539 - Flags: superreview?(bzbarsky)
Attachment #359539 - Flags: review?(timeless)
Comment on attachment 359641 [details] [diff] [review]
proper style

I can't review js/jsd patches, only Vnk internals. Leaving it to you to pick someone else. Sorry!
Attachment #359641 - Flags: review?(gijskruitbosch+bugs) → review?
Comment on attachment 359641 [details] [diff] [review]
proper style

as for the l10n stuff, it makes the api usable, who would we want to discuss with about making that change?

i generally don't care about branches, if you're my consumer and want the stuff to work and are willing to ask for it to go back to 1.9.1, i'm fine w/ working or supporting the backport.
Attachment #359641 - Flags: review? → review?(caillon)
Attachment #359641 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 359641 [details] [diff] [review]
proper style

This looks ok if you keep the AUTF8String thing.
This is a change we're making on 1.9.1? We'd need to document it with late-compat, although it's unlikely very many extension authors are using this API. Is firebug?
(In reply to comment #13)
> Created an attachment (id=359641) [details]
> proper style
> 
> the patch that added this function wasn't given sr. and should never have been
> given sr. It violates a number of rules. interCaps method naming, adding
> methods at the end.

You're so right -- how did I miss all that? I re-read the patch just a few minutes ago and still missed the IDL. Read the raw diff and saw it. Sorry,


> Since we're reving the interface anyway, i'd like to fix the name and location.
> We're breaking any consumer anyway, and the js ones can handle function case
> naming if they care (we did it for nsIRunnable years ago).

Please fix up the name and location. Thanks,

/be
(In reply to comment #17)
> This is a change we're making on 1.9.1? We'd need to document it with
> late-compat, although it's unlikely very many extension authors are using this
> API. Is firebug?

Yes this API is the basis for firebug js debugging (and venkman). A few others use it for odd purposes. 

Other than disagreeing with the design decision to add a method like dumpHeap() to this API, the change would not seem to affect Firebug in any way.
johnjbarton: I can't tell from comment 19 whether you are or aren't using the dumpHeap API...
Oh, sorry I didn't understand the scope of your first question. We don't use dumpHeap, don't even know what it does.
We could take dumpHeap out. Wouldn't that be better?

Igor, why did you add it? I completely missed it, so never asked.

/be
(In reply to comment #22)
> We could take dumpHeap out. Wouldn't that be better?
> 
> Igor, why did you add it? I completely missed it, so never asked.

Before the patch for the bug 378261 jsdService::GC (implementing jsdIDebuggerService::GC) was printing the GC heap if GC_MARK_DEBUG was defined using js_DumpGCHeap variable. Since the patch replaced that by a separated function, I added DumpHeap to the interface so the users of the interface can still get heap dumping functionality.
As per today's meeting, want to get all the UUID revs in for b3 ...
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Comment on attachment 359641 [details] [diff] [review]
proper style

r=bzbarsky too, for what it's worth.  Please land on m-c and 1.9.1?
Attachment #359641 - Flags: review+
Attached patch for checkinSplinter Review
I know I'm supposed to be a better checkin slave than this, but the constant tinderbox failures are defeating my every attempt to push anything.
Attachment #359641 - Attachment is obsolete: true
Attachment #359641 - Flags: review?(caillon)
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1b3
Version: unspecified → Trunk
Pushed http://hg.mozilla.org/mozilla-central/rev/8c55fb9f075b and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fb4b5058c2fb
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
Product: Other Applications → Other Applications Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: