Last Comment Bug 731563 - Markers in places popup views are broken and too much bugs-prone, causing broken queries
: Markers in places popup views are broken and too much bugs-prone, causing bro...
Status: RESOLVED FIXED
: addon-compat, regression
Product: Toolkit
Classification: Components
Component: Places (show other bugs)
: 13 Branch
: All All
: -- normal (vote)
: mozilla13
Assigned To: Marco Bonardo [::mak]
:
: Marco Bonardo [::mak]
Mentors:
: 730798 (view as bug list)
Depends on: 734646 734653 753672
Blocks: livemarksIO
  Show dependency treegraph
 
Reported: 2012-02-29 05:55 PST by Alice0775 White
Modified: 2012-05-10 00:56 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1.0 (17.48 KB, patch)
2012-03-01 13:28 PST, Marco Bonardo [::mak]
no flags Details | Diff | Splinter Review
patch v1.1 (23.12 KB, patch)
2012-03-01 14:52 PST, Marco Bonardo [::mak]
no flags Details | Diff | Splinter Review
patch v1.0 (27.42 KB, patch)
2012-03-06 10:12 PST, Marco Bonardo [::mak]
asaf: review+
Details | Diff | Splinter Review
patch v1.1 (27.43 KB, patch)
2012-03-06 11:22 PST, Marco Bonardo [::mak]
no flags Details | Diff | Splinter Review

Description Alice0775 White 2012-02-29 05:55:11 PST
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/7ce4d9b55863
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120228 Firefox/13.0a1 ID:20120228031102

Smart Bookmarks is broken.
After click reload, Items are doubled.

Reproducible: Always


Steps to Reproduce:
1. Start Firefox with new profile
2. App bottun > Bokmarks > Bookmarks – Recently Bookmarked
   -- remember number of items.
3. Clcik reload button
4. App bottun > Bokmarks > Bookmarks – Recently Bookmarked


Actual Results:
  Items are doubled.

Expected Results:
  should not

Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/2b1a53905350
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120224 Firefox/13.0a1 ID:20120224133104
Fails:
http://hg.mozilla.org/mozilla-central/rev/ce20e9b47e9c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225031723
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2b1a53905350&tochange=ce20e9b47e9c


Regression window(m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e2ee770cc461
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120224 Firefox/13.0a1 ID:20120224041330
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/7925703fbce6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120224 Firefox/13.0a1 ID:20120224044430
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e2ee770cc461&tochange=7925703fbce6

Suspected: Bug 613588
Comment 1 Alice0775 White 2012-02-29 06:00:00 PST
@jrudkin also reports in mozillazine http://forums.mozillazine.org/viewtopic.php?p=11782043#p11782043
Comment 2 Marco Bonardo [::mak] 2012-02-29 06:02:17 PST
thanks, I can reproduce
Comment 3 Marco Bonardo [::mak] 2012-03-01 13:28:10 PST
Created attachment 602086 [details] [diff] [review]
patch v1.0

oviously what broke is the markers, as usual.
I've seen far too many bogus behaviors with them, so I finally decided to do what I planned from quite some time, rewrite them as hidden elements. This way they just move when adding or removing other content, and we can insertBefore them.

Mano, I still have to test this on Mac, though asking review cause this builds on top of bug 730798 and I don't want to confuse you.
Comment 4 Marco Bonardo [::mak] 2012-03-01 13:32:45 PST
Comment on attachment 602086 [details] [diff] [review]
patch v1.0

ehr, indeed this is more a feedback than a review, not ready for review.
Comment 5 Marco Bonardo [::mak] 2012-03-01 13:55:26 PST
good news, this fixed some of the brokeness I saw in bug 730798, though some popups show scrollarows on mac when there should not be.
Comment 6 Marco Bonardo [::mak] 2012-03-01 14:52:06 PST
Created attachment 602128 [details] [diff] [review]
patch v1.1

tested on win and mac and passes our b-c tests. I coulnd't ask more.
Comment 7 Marco Bonardo [::mak] 2012-03-06 05:31:57 PST
Comment on attachment 602128 [details] [diff] [review]
patch v1.1

a couple bugs still, updating patch
Comment 8 Marco Bonardo [::mak] 2012-03-06 10:12:20 PST
Created attachment 603330 [details] [diff] [review]
patch v1.0

includes most of the stuff discussed on irc
Comment 9 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2012-03-06 10:38:29 PST
Comment on attachment 603330 [details] [diff] [review]
patch v1.0

r=mano. nits:

1. Move the markers s/at/to/  the right position.
2. Please referenced the now-filed relevant mac menu bug,
3. It's name is "OS X" ;)

I mean, r=mano. wow.
Comment 10 Marco Bonardo [::mak] 2012-03-06 11:22:18 PST
Created attachment 603374 [details] [diff] [review]
patch v1.1
Comment 12 Marco Bonardo [::mak] 2012-03-06 11:30:32 PST
*** Bug 730798 has been marked as a duplicate of this bug. ***
Comment 13 Marco Bonardo [::mak] 2012-03-07 02:36:43 PST
https://hg.mozilla.org/mozilla-central/rev/5aa498cf8658
Comment 14 Marco Bonardo [::mak] 2012-03-08 04:44:59 PST
This may have some effect on add-ons that wrongly use private _startMarker and _endMarker properties of Places popups, or directly use the childNodes count. The change is that instead of being numeric indices, those are now elements of the popup. So adding the keyword.

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