361694 - Altering a tag's importance resets its name and colour

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[[e-mail]]

Steps to reproduce problem:
1. Change the name and colour of a tag
2. Make it more or less importnant

Actual result: tag name and colour is reset to its original value

Workaround: change tag importance first, or switch pref panes

Comment 1

[Karsten Düsterloh]

Attachment: trunk patch

Read the possibly changed values before moving the elements.

Comment 2

[Karsten Düsterloh]

Attachment: 1.8 branch patch

Same as the trunk patch, but with an additional fix for a focus loss when moving the listitems.

Comment 3

[[e-mail]]

Comment on attachment 246485 [details] [diff] [review]
1.8 branch patch

> function FocusTagEntry(aEntry)
> {
>+  OnFocus({target: aEntry});
>   // focus the entry's textbox
>   gTagList.ensureElementIsVisible(aEntry);
>   aEntry.firstChild.firstChild.focus();
> }
This is wrong for two reasons:
a) it fixes the the focus loss before it was supposed to happen
b) replicating the focus event itself isn't the correct fix,
   the correct fix is to replicate the effect of the event handler.

Comment 4

[Karsten Düsterloh]

Landed trunk fix.

Comment 5

[Karsten Düsterloh]

Attachment: 1.8 branch patch, v2

> >+  OnFocus({target: aEntry});
> >   // focus the entry's textbox
> >   gTagList.ensureElementIsVisible(aEntry);
> >   aEntry.firstChild.firstChild.focus();
> > }
> This is wrong for two reasons:
> a) it fixes the the focus loss before it was supposed to happen

Uh? The focus loss of the listbox happens when MoveTag calls .insertBefore to move the listitem (just like on trunk). Only on trunk, the textbox' .focus() call in FocusTagEntry will select the listitem, but doesn't on branch.

> b) replicating the focus event itself isn't the correct fix,
>    the correct fix is to replicate the effect of the event handler.

Yeah. Here's a less hackier version, just doing the missing listitem selection before focussing the textbox.

Comment 6

[[e-mail]]

Comment on attachment 246533 [details] [diff] [review]
1.8 branch patch, v2

Sorry I forgot to debug this before, and smaug didn't answer his memo, but the actual problem is that removing the element from the dom kills the listener.

Comment 7

[Karsten Düsterloh]

Attachment: 1.8 branch patch, v3

- recreate the lost event listener
- fix missing label/color update for items not in view
- fix button id typo in .xul

Comment 8

[[e-mail]]

Comment on attachment 246539 [details] [diff] [review]
1.8 branch patch, v3

Also to move the call to UpdateTagEntry on the trunk (we could land that with the onfocus comments and optimization patch if you like).

Comment 9

[Karsten Düsterloh]

Attachment: additional trunk patch

Landed additional fixes (moved call, .xul id) on trunk as per comment #8.

Comment 10

[Robert Kaiser]

Comment on attachment 246539 [details] [diff] [review]
1.8 branch patch, v3

first-a=me for SeaMonkey 1.1, one more needed

Comment 11

[Karsten Düsterloh]

Landed on MOZILLA_1_8_BRANCH.