Closed Bug 351479 Opened 19 years ago Closed 19 years ago

nsIChangeObserver scriptability change breaks JavaXPCOM on 1.8.1 branch

Categories

(Core Graveyard :: Widget: Mac, defect)

Product:
Core Graveyard
Component:
Widget: Mac
Version:
1.8 Branch
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: jhpedemonte)

Details

(Keywords: fixed1.8.1, Whiteboard: [waiting on answer to Darin's question])

Attachments

(1 file, 1 obsolete file)

patch
1.53 KB, patch
jaas
: review+
mtschrep
: approval1.8.1+
Details | Diff | Splinter Review
Building patch from bug 333618 on 1.8.1 branch, and found an issue with nsIChangeObserver. On the 1.8.1 branch, nsIChangeObserver is marked as non-scriptable, but nsIChangeManager (which is 'scriptable'), uses nsIChangeObserver. This breaks JavaXPCOM on the 1.8.1 branch. I noticed that on the trunk, both interfaces are marked as 'scriptable'. So should we make nsIChangeObserver 'scriptable' again?
Forgot to mention that the change to nsIChangeObserver is discussed in bug 50590.
Attached patch 1.8.1 branch patch (obsolete) — — Splinter Review
Revert scriptability change and bring in line with trunk.
Attachment #236957 - Flags: review?(joshmoz)
Attachment #236957 - Flags: review?(joshmoz) → review+
Comment on attachment 236957 [details] [diff] [review] 1.8.1 branch patch Asking for 1.8.1 branch approval. This patch reverts an inadvertant change made a month ago, and brings the branch in line with the trunk.
Attachment #236957 - Flags: approval1.8.1?
Should the methods be marked non-scriptable because nsIContent and nsIDocument are not exactly scriptable.
Whiteboard: [waiting on answer to Darin's question]
Bloody hell, I hadn't noticed that all the methods for both interfaces take nsIContent or nsIDocument. So I guess the correct fix would be to make both interfaces non-scriptable, since neither is useful to the 'scriptable' languages. Darin, would you be fine taking that change on the 1.8.1 branch?
Attached patch patch — — Splinter Review
Based on comment #4 and comment #5, there's not much reason for these interfaces to be scriptable. This patch removes that. We'll see if this is acceptable on the 1.8.1 branch.
Attachment #236957 - Attachment is obsolete: true
Attachment #238073 - Flags: review?(joshmoz)
Attachment #236957 - Flags: approval1.8.1?
Comment on attachment 238073 [details] [diff] [review] patch this should be fine for 1.8.1 branch, doing this won't break anything for anyone
Attachment #238073 - Flags: review?(joshmoz) → review+
Attachment #238073 - Flags: approval1.8.1?
Comment on attachment 238073 [details] [diff] [review] patch a=schrep for 181drivers
Attachment #238073 - Flags: approval1.8.1? → approval1.8.1+
Assignee: joshmoz → jhpedemonte
Checked in to trunk and 1.8.1 branch. ->FIXED
Status: NEW → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: