Closed
Bug 405044
Opened 17 years ago
Closed 17 years ago
clean up updates.xml
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Gavin
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
updates.xml has two "update" bindings and something strange near
the end of file. See lines 10,340,390
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/update/content/updates.xml&rev=1.33&mark=10,340,390
Can't fix bug 401907 before this.
Assignee | ||
Comment 1•17 years ago
|
||
Is the "update" binging even used anywhere?
(So great that updates.xml is probably mostly unreviewed code and blame doesn't
point to any bugs.)
Anyone familiar with updater code?
Comment 2•17 years ago
|
||
It would sure be easier to know if the original bugs had patches, let alone were reviewed :(
It seems to be used by: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/update/content/history.js&rev=1.7&mark=64#64
Assignee | ||
Comment 3•17 years ago
|
||
This might be the right patch, but I don't really know how to test this.
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 289937 [details] [diff] [review]
possible patch
Tested using updates.xml.
With the patch UI looks the same. If the second binding is removed and first one kept, UI changes.
Attachment #289937 -
Flags: review?(gavin.sharp)
Updated•17 years ago
|
Attachment #289937 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 5•17 years ago
|
||
Comment on attachment 289937 [details] [diff] [review]
possible patch
Bug 401907 needs this one fixed.
Attachment #289937 -
Flags: approval1.9?
Comment 6•17 years ago
|
||
Comment on attachment 289937 [details] [diff] [review]
possible patch
a=beltzner for drivers
Attachment #289937 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Assignee: nobody → Olli.Pettay
Comment 7•17 years ago
|
||
> If the second binding is removed and first one kept, UI changes.
questions:
should we be concerned that the "last binding wins"? (then again, if you are able to provide an xbl binding, the horse is out of the barn.)
Would it be worth logging a xbl bug about checking if we have an existing binding to ignore any other bindings (and assert?)
Could someone write a theme and and override an existing binding? (I forget what themes can and can't do.)
do we (or extension authors) rely on "last binding wins" to purposefully override bindings?
forgive the private comment. if this isn't legitimately security sensitive, we can mark it un-private and reveal my ignorance!
(In reply to comment #7)
> > If the second binding is removed and first one kept, UI changes.
>
> questions:
>
> should we be concerned that the "last binding wins"? (then again, if you are
> able to provide an xbl binding, the horse is out of the barn.)
I can't think of a case where this would be a problem, other than that it could be a source of bugs like any somewhat unexpected/ambiguous behavior in a programming language.
> Would it be worth logging a xbl bug about checking if we have an existing
> binding to ignore any other bindings (and assert?)
Yes, I think it would be a good idea to assert.
In general we shouldn't assert on errors that end-users can cause. However the XBL code is full of such assertions. Ideally we should change these to assert only for chrome-xbl, and just log to the error console for http-xbl. But that's a separate bug.
> Could someone write a theme and and override an existing binding? (I forget
> what themes can and can't do.)
Yes. You can simply write a new rule in one of the stylesheets and override the -moz-binding CSS property. This is just how XBL1 works unfortunately.
> do we (or extension authors) rely on "last binding wins" to purposefully
> override bindings?
The "last binding wins" rule is only within a single XBL file. So I don't think this is a problem in any way.
> forgive the private comment. if this isn't legitimately security sensitive,
> we can mark it un-private and reveal my ignorance!
I think we can unmark it
Comment 9•17 years ago
|
||
jonas, thanks for answering all my questions (and more).
> I think it would be a good idea to assert.
I've logged bug #405563
Assignee | ||
Updated•17 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•