Closed
Bug 776399
Opened 12 years ago
Closed 12 years ago
undo incompatible IDL changes done in bug 663057
Categories
(Core Graveyard :: File Handling, defect)
Core Graveyard
File Handling
Tracking
(firefox15+ fixed, firefox16+ fixed)
RESOLVED
FIXED
mozilla17
People
(Reporter: julian.reschke, Assigned: julian.reschke)
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The changes for bug 663057 have changed the IDL for nsIMIMEHeaderParam incompatibly.
Comment 1•12 years ago
|
||
Julian, do you have time to deal with this, or should I?
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #1)
> Julian, do you have time to deal with this, or should I?
I'm currently checking out beta/aurora/central, and try to fix and test all of these tomorrow. Hopefully the same patch will work on all of these.
Assignee | ||
Comment 3•12 years ago
|
||
try results: https://tbpl.mozilla.org/?tree=Try&rev=e21ac319d8b6
Attachment #644867 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•12 years ago
|
||
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 776399
User impact if declined: n/a (IDL change backed out)
Testing completed (on m-c, etc.): https://tbpl.mozilla.org/?tree=Try&rev=e21ac319d8b6 (m-c)
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #644876 -
Flags: review?(bzbarsky)
Attachment #644876 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 5•12 years ago
|
||
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 776399
User impact if declined: n/a (IDL change backed out)
Testing completed (on m-c, etc.): https://tbpl.mozilla.org/?tree=Try&rev=e21ac319d8b6 (m-c)
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #644877 -
Flags: review?(bzbarsky)
Attachment #644877 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 6•12 years ago
|
||
Posted patches for central/aurora/beta separately (although the changes identical; dunno what's better process-wise).
Updated•12 years ago
|
Comment 7•12 years ago
|
||
No, this doesn't work. At this point this would be an incompatible change from what we've already shipped on aurora/beta, which is just as bad, if not worse.
Again, I think the right way to make this work is to make aAllowSubstitution an optional argument and use optional_argc to allow it to default to true...
Comment 8•12 years ago
|
||
Comment on attachment 644867 [details] [diff] [review]
Proposed patch (mozilla-central)
Why are we making this exact change?
(Also, for future reference, uploading the one patch and setting the approval flags on it would have been fine.)
Attachment #644867 -
Flags: review?(bzbarsky) → review+
Comment 9•12 years ago
|
||
Comment on attachment 644876 [details] [diff] [review]
Proposed patch (aurora)
r- for Aurora.
Attachment #644876 -
Flags: review?(bzbarsky) → review-
Comment 10•12 years ago
|
||
Comment on attachment 644877 [details] [diff] [review]
Proposed patch (beta)
r- for beta
Attachment #644877 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 11•12 years ago
|
||
Boris, you confused me with comments 7 and 8. If this is not the right thing to do, why r+ for the change in mozilla-central?
Comment 12•12 years ago
|
||
Comment on attachment 644867 [details] [diff] [review]
Proposed patch (mozilla-central)
Er, I meant to just unset the flag, not set it to +. ;)
Attachment #644867 -
Flags: review+
Assignee | ||
Comment 13•12 years ago
|
||
A patch that makes the new parameter optional, and lets it default to "true".
Attachment #644867 -
Attachment is obsolete: true
Attachment #645281 -
Flags: review?(bzbarsky)
Comment 14•12 years ago
|
||
Comment on attachment 645281 [details] [diff] [review]
Proposed patch
s/aArgc/aOptionalArgc/ and r=me
Attachment #645281 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #645281 -
Attachment is obsolete: true
Attachment #645309 -
Flags: review?(bzbarsky)
Comment 16•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
r=me
Attachment #645309 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 17•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
Try submission: https://tbpl.mozilla.org/?tree=Try&rev=357d0d07d1fc
Assignee | ||
Updated•12 years ago
|
Attachment #645309 -
Flags: checkin?
Assignee | ||
Comment 18•12 years ago
|
||
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 663057
User impact if declined: n/a
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): n/a
String or UUID changes made by this patch:
Attachment #644876 -
Attachment is obsolete: true
Attachment #644876 -
Flags: approval-mozilla-aurora?
Attachment #645413 -
Flags: review?(bzbarsky)
Attachment #645413 -
Flags: approval-mozilla-aurora?
Comment 19•12 years ago
|
||
Isn't that the same patch? Just ask for approval on the already-existing patch, if so.
Assignee | ||
Comment 20•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #19)
> Isn't that the same patch? Just ask for approval on the already-existing
> patch, if so.
OK, will do.
Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 663057
User impact if declined: n/a
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): n/a
String or UUID changes made by this patch:
Attachment #645309 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 22•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 663057
User impact if declined: n/a
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): n/a
String or UUID changes made by this patch:
Attachment #645309 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 23•12 years ago
|
||
Julian, assigning this to you so we don't have a tracked 15/16 bug with no one assigned to it. We'll wait for this to land on mozilla-central first before looking at approving for aurora/beta.
Assignee: nobody → julian.reschke
Updated•12 years ago
|
Attachment #644877 -
Flags: approval-mozilla-beta?
Comment 24•12 years ago
|
||
Comment on attachment 645413 [details] [diff] [review]
Proposed patch (aurora)
(cleaning up unnecessary approval flags)
Attachment #645413 -
Flags: approval-mozilla-aurora?
Comment 25•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
Just use checkin-needed.
Attachment #645309 -
Flags: checkin?
Comment 26•12 years ago
|
||
Flags: in-testsuite-
Keywords: checkin-needed
Comment 27•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 28•12 years ago
|
||
Comment on attachment 645309 [details] [diff] [review]
Proposed patch
[Triage Comment]
Now that Comment 7 has been addressed, approving for branches.
Attachment #645309 -
Flags: approval-mozilla-beta?
Attachment #645309 -
Flags: approval-mozilla-beta+
Attachment #645309 -
Flags: approval-mozilla-aurora?
Attachment #645309 -
Flags: approval-mozilla-aurora+
Comment 29•12 years ago
|
||
Can someone land this on Julian's behalf to the branches?
status-firefox15:
--- → affected
status-firefox16:
--- → affected
Comment 30•12 years ago
|
||
Do the branch patches have r+? If so, put checkin-needed in the whiteboard.
Updated•12 years ago
|
Attachment #645413 -
Attachment is obsolete: true
Attachment #645413 -
Flags: review?(bzbarsky)
Comment 32•12 years ago
|
||
Comment on attachment 644877 [details] [diff] [review]
Proposed patch (beta)
In that case, marking obsolete patches as such is helpful.
Attachment #644877 -
Attachment is obsolete: true
Comment 33•12 years ago
|
||
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•