418216 - Back button greys when toolbar reset to default

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P3)

Description

[Ben Wishaw]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre

When the toolbar is reset to defaults the back navigation button becomes greyed. The drop down list is still available though. If a new tab is opened and navigated with the the back button becomes reactivated in the new tab and the one it was in when greyed initially.

Reproducible: Always

Steps to Reproduce:
To break
1.Right click and select customize toolbar
2. Click restore default set button

To fix
1. Open new tab
2. use it to navigate the web
Actual Results:  
To break - the back button became greyed
To fix - the back button returned to green

Expected Results:  
The back button to remain green

This was a fresh install of minefield with no add ons or plug ins.

Comment 1

[Ben Wishaw]

This does not occur in Ubuntu - Gutsy

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020507 Firefox/3.0b3pre

Comment 2

[Ria Klaassen (not reading all bugmail)]

Confirmed with latest trunk on Windows XP.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Range:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1202619780&maxdate=1202621819

Comment 4

[arno renevier]

Attachment: testcase

when toolbar is created, toolbar.palette gets a reference to back-button, then back-button is removed from document.

When toolbar is reset, previously saved back-button is inserted.

Unfortunately, if Browser:Back "disabled" attribute has been deleted, that removal is not transfered to back-button.

Comment 5

[arno renevier]

Attachment: patch v1

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 304920 [details] [diff] [review]
patch v1

Neil, does this approach look right?

Comment 7

[Neil Deakin]

Comment on attachment 304920 [details] [diff] [review]
patch v1

>+        } else {
>+            listener->UnsetAttr(kNameSpaceID_None, name,
>+                              mInitialLayoutComplete);

The indenting should line up.

>+<box id="container">
>+<textbox id="item">
>+    <observes element="broadcaster" attribute="disabled"/>
>+</textbox>
>+</box>

It would also be good to test other forms for observing as well, like the observes or command attributes.

Going to accept this, but let's get another Neil to superreview.

Comment 8

[[e-mail]]

Comment on attachment 304920 [details] [diff] [review]
patch v1

Whoops, that got overlooked in the fix for bug 414907 :-(

It's a good thing toolkit's sucky toolbar customisation code removes nodes from the DOM otherwise nobody would have noticed ;-)

Comment 9

[arno renevier]

> It would also be good to test other forms for observing as well, like the
> observes or command attributes.

With an <observe>, you can check if observed attribute has been removed from
broadcaster. With a <command> or <broadcaster>, as "*" is observed, you cannot
known if a specific attribute was present before, and has been removed.

Comment 10

[arno renevier]

Attachment: patch v1.1

same patch, identation modified as requested in comment 7.
carrying over r and sr

Comment 11

[Reed Loden [:reed]]

Checking in content/xul/document/src/nsXULDocument.cpp;
/cvsroot/mozilla/content/xul/document/src/nsXULDocument.cpp,v  <--  nsXULDocument.cpp
new revision: 1.809; previous revision: 1.808
done
Checking in content/xul/document/test/Makefile.in;
/cvsroot/mozilla/content/xul/document/test/Makefile.in,v  <--  Makefile.in
new revision: 1.7; previous revision: 1.6
done
RCS file: /cvsroot/mozilla/content/xul/document/test/test_bug418216.xul,v
done
Checking in content/xul/document/test/test_bug418216.xul;
/cvsroot/mozilla/content/xul/document/test/test_bug418216.xul,v  <--  test_bug418216.xul
initial revision: 1.1
done

Comment 13

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre. Also verified on Win XP.