jsdIDebuggerService.idl changed without changing uuid

RESOLVED FIXED in mozilla1.9.1b3

Status

Other Applications
Venkman JS Debugger
P1
major
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: palo misik, Assigned: timeless)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.1b3
fixed1.9.1
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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
(Reporter)

Updated

9 years ago
Flags: wanted-thunderbird3?
Assignee: nobody → rginda
Component: General → JavaScript Debugger
Flags: wanted-thunderbird3?
Product: Thunderbird → Other Applications
QA Contact: general → venkman

Comment 1

9 years ago
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

Comment 2

9 years ago
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.
(Reporter)

Comment 3

9 years ago
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.
(Reporter)

Comment 4

9 years ago
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.

Comment 5

9 years ago
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...

Comment 8

9 years ago
(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...

Comment 9

9 years ago
Given that this is in a prior stable release, I think it should be WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 9 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.

Comment 12

9 years ago
Created attachment 359539 [details] [diff] [review]
Patch for 1.9.1

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)
(Assignee)

Comment 13

9 years ago
Created attachment 359641 [details] [diff] [review]
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.

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 14

9 years ago
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?
(Assignee)

Comment 15

9 years ago
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.

Comment 17

9 years ago
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

Comment 19

9 years ago
(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.

Comment 20

9 years ago
johnjbarton: I can't tell from comment 19 whether you are or aren't using the dumpHeap API...

Comment 21

9 years ago
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

Comment 23

9 years ago
(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+
Created attachment 360667 [details] [diff] [review]
for checkin

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
Last Resolved: 9 years ago9 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.