Closed
Bug 282190
Opened 20 years ago
Closed 18 years ago
sync xpfe tree.xml with toolkit tree.xml
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
WONTFIX
mozilla1.9alpha1
People
(Reporter: mconnor, Unassigned)
References
(Depends on 1 open bug)
Details
(Whiteboard: [bug 272059 should fix most/all of this bug :-)])
Attachments
(2 files, 1 obsolete file)
17.48 KB,
patch
|
Details | Diff | Splinter Review | |
1.85 KB,
patch
|
mnyromyr
:
review+
|
Details | Diff | Splinter Review |
Comment 1•20 years ago
|
||
This is another one that can just be taken straight from toolkit. Only changes
made in the toolkit version is s/control/accel/ and an ifdef instead of
"!/Mac/.test(navigator.platform)".
Comment 2•20 years ago
|
||
Comment 3•20 years ago
|
||
Patch in bug 272059
Updated•20 years ago
|
Whiteboard: [bug 272059 should fix most/all of this bug :-)]
Comment 4•19 years ago
|
||
The difference is not that big. Mano's patch in bug 263146 was never ported to the xpfe version. Those changes shouldn't be any problem, though - in fact, xpfe needs them ;). Then there are a few other differences.
Attachment #175223 -
Attachment is obsolete: true
![]() |
||
Comment 5•19 years ago
|
||
from what I see in a fast look over the file, we should probably just pack toolkit's version in suite and forget about real sync'ing in this case...
![]() |
||
Comment 6•19 years ago
|
||
I propose to fix this file once and for all (for SeaMonkey) by just using the toolkit file in suite's toolkit.jar
I've taken the conservative approach there though, leaving a non-suite xpfe build alone (wherever they still may exist) and only touching suite builds.
I propose to take this approach for the whole migration of suite widgetry to the new toolkit, as it guarantees us not to break anyone else who might still rely on the old xpfe toolkit.jar - Camino still might do that, and while this tree.xml fix would fit for them, others we switch the same way might not, but we still can add those switches into that new #ifdef block.
Attachment #216235 -
Flags: superreview?(neil)
Attachment #216235 -
Flags: review?(mnyromyr)
Updated•19 years ago
|
Attachment #216231 -
Attachment description: Current diff (2006060325) → Current diff (20060325)
Comment 7•19 years ago
|
||
Unfortunately there are at least two bugs with toolkit's tree.xml
The first is that the current index is not allowed to be -1
This breaks the mail thread pane
The second is that you can't accel-click rows in BeOS
Fortunately that will be fixed by bug 302174
![]() |
||
Comment 8•19 years ago
|
||
Neil, are those bugs fixed in our current implementation?
If yes, is there a reasonable chance to backport those fixes?
And did you point to the correct bug for the second one? If so, that's a dupe obviously...
![]() |
||
Comment 9•19 years ago
|
||
(In reply to comment #7)
> Unfortunately there are at least two bugs with toolkit's tree.xml
>
> The first is that the current index is not allowed to be -1
> This breaks the mail thread pane
From what I see, Thunderbird uses the toolkit version and doesn't break...
> The second is that you can't accel-click rows in BeOS
> Fortunately that will be fixed by bug 302174
I can't follow this one, but that shouldn't block reviews for this patch (if we still want to go down that road anyways), esp. if there's a fix for it.
Comment 10•19 years ago
|
||
(In reply to comment #6)
> Created an attachment (id=216235) [edit]
> package toolkit's tree.xml into suite's toolkit.jar
>
> I've taken the conservative approach there though, leaving a non-suite xpfe
> build alone (wherever they still may exist) and only touching suite builds.
>
> I propose to take this approach for the whole migration of suite widgetry to
> the new toolkit, as it guarantees us not to break anyone else who might still
> rely on the old xpfe toolkit.jar - Camino still might do that, and while this
> tree.xml fix would fit for them, others we switch the same way might not, but
> we still can add those switches into that new #ifdef block.
On <http://wiki.mozilla.org/SeaMonkey:Toolkit_Transition#Status_of_each_widget>,
I read "Camino just has embed.jar and pipnss.jar, so no problems here.";
then, maybe the if switch is not needed ?
(Though I don't know either if there is any other "non-suite xpfe build")
Comment 11•19 years ago
|
||
(In reply to comment #10)
> On
> <http://wiki.mozilla.org/SeaMonkey:Toolkit_Transition#Status_of_each_widget>,
(Rather <http://wiki.mozilla.org/SeaMonkey:Toolkit_Transition#XUL_Widgets>.)
Comment 12•19 years ago
|
||
> > The first is that the current index is not allowed to be -1
> > This breaks the mail thread pane
>
> From what I see, Thunderbird uses the toolkit version and doesn't break...
The problem is that the toolkit tree does not allow the tree to have the focus without also having a row selected; it will automatically select the first row in this case:
- <handler event="focus" action="this.treeBoxObject.focused = true;"/>
+ <handler event="focus">
+ <![CDATA[
+ this.treeBoxObject.focused = true;
+ if (this.currentIndex == -1 && this.view.rowCount > 0) {
+ this.currentIndex = this.treeBoxObject.getFirstVisibleRow();
+ }
+ ]]>
+ </handler>
Neil, what do mean by "break"?
Comment 13•19 years ago
|
||
Comment on attachment 216235 [details] [diff] [review]
package toolkit's tree.xml into suite's toolkit.jar
>Index: mozilla/xpfe/global/jar.mn
>===================================================================
> content/global/bindings/toolbarbutton.xml (resources/content/bindings/toolbarbutton.xml)
>- content/global/bindings/tree.xml (resources/content/bindings/tree.xml)
> content/global/bindings/wizard.xml (resources/content/bindings/wizard.xml)
>+#ifdef MOZ_SUITE
>+*+ content/global/bindings/tree.xml (/toolkit/content/widgets/tree.xml)
>+#else
>+ content/global/bindings/tree.xml (resources/content/bindings/tree.xml)
>+#endif
The ifdef isn't necessary, not even Camino is using this.
And you should keep the alphabetical order of the bindings. ;-)
r=me with that
Attachment #216235 -
Flags: review?(mnyromyr) → review+
Comment 14•19 years ago
|
||
(In reply to comment #12)
>Neil, what do mean by "break"?
Bug 305140. Bug 304565. Bug 304676 comment 5. Bug 302516 comment 14.
Comment 15•19 years ago
|
||
Setting open toolkit tree.xml problems as blocking.
![]() |
||
Comment 16•18 years ago
|
||
I don't think there's anything we still need to do here, get rid of this bug.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WONTFIX
![]() |
||
Updated•18 years ago
|
Attachment #216235 -
Flags: superreview?(neil)
You need to log in
before you can comment on or make changes to this bug.
Description
•