Last Comment Bug 726440 - The star panel should not replace the header with itself each time it's opened
: The star panel should not replace the header with itself each time it's opened
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 13
Assigned To: Marco Bonardo [::mak]
:
: Marco Bonardo [::mak]
Mentors:
Depends on: 725784
Blocks: 638785 647603 763708
  Show dependency treegraph
 
Reported: 2012-02-12 09:10 PST by Alice0775 White
Modified: 2012-06-11 14:33 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
possible fix (1.32 KB, patch)
2012-02-12 09:33 PST, Alice0775 White
no flags Details | Diff | Splinter Review
patch v1.0 (2.97 KB, patch)
2012-02-23 08:44 PST, Marco Bonardo [::mak]
dietrich: review+
Details | Diff | Splinter Review

Description Alice0775 White 2012-02-12 09:10:13 PST
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/d71dab82fff4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120211 Firefox/13.0a1 ID:20120211031145

Header label of "Edit bookmarks panel" does not change till restart browser.
The header label should be "Page Bookmarked" or "Edit This Bookmark" depending on state of bookmark.

Reproducible: Always

Steps to Reproduce#1:
1. Start Firefox with new profile
2. Open non bookmarked page
3. CTRL+D  ---- you see "Page Bookmarked" as expected
4, Double Click STAR UI

Actual Results:
  The header label is "Page Bookmarked"

Expected Results:
  The header label should be "Edit This Bookmark"


Steps to Reproduce#2:
1. Start Firefox with new profile
2. Open a bookmarked page
3. Double Click STAR UI  ---- you see "Edit This Bookmark" as expected
4. Open non bookmarked page
5. CTRL+D


Actual Results:
  The header label is "Edit This Bookmark"

Expected Results:
  The header label should be "Page Bookmarked"

Regression window(cachedm-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/7aa58a99a2e5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 ID:20110630003937
Fails:
http://hg.mozilla.org/mozilla-central/rev/5c246f2bccb1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 ID:20110630035829
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7aa58a99a2e5&tochange=5c246f2bccb1


Regression window(cached m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/6777320f6f29
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110629 Firefox/7.0a1 ID:20110629215351
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c2f48684b9f5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110629 Firefox/7.0a1 ID:20110629231921
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6777320f6f29&tochange=c2f48684b9f5


In local build:
first bad changeset c2f48684b9f5
last good changeset fb03584dd823
Comment 1 Alice0775 White 2012-02-12 09:33:05 PST
Created attachment 596493 [details] [diff] [review]
possible fix

using setAttribute to set value strings
Comment 2 Marco Bonardo [::mak] 2012-02-23 08:24:08 PST
Comment on attachment 596493 [details] [diff] [review]
possible fix

Will shortly attach a patch that avoid the self replacement, since it was a no-op but now it's not anymore. We don't need to pay that cost.

Btw, would like to thank you for finding these bugs and the problem.
Comment 3 Marco Bonardo [::mak] 2012-02-23 08:44:36 PST
Created attachment 600035 [details] [diff] [review]
patch v1.0

So basically, instead of moving it each time the panel is opened, this moves it when the binding is lazy loaded on first opening.
Comment 5 Marco Bonardo [::mak] 2012-02-24 09:25:45 PST
https://hg.mozilla.org/mozilla-central/rev/4e2e861f136b

Note You need to log in before you can comment on or make changes to this bug.