Closed Bug 731563 Opened 9 years ago Closed 9 years ago

Markers in places popup views are broken and too much bugs-prone, causing broken queries

Categories

(Toolkit :: Places, defect)

13 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: alice0775, Assigned: mak)

References

Details

(Keywords: addon-compat, regression)

Attachments

(1 file, 3 obsolete files)

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
@jrudkin also reports in mozillazine http://forums.mozillazine.org/viewtopic.php?p=11782043#p11782043
thanks, I can reproduce
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Attached patch patch v1.0 (obsolete) — Splinter Review
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.
Attachment #602086 - Flags: review?(mano)
Depends on: 730798
Comment on attachment 602086 [details] [diff] [review]
patch v1.0

ehr, indeed this is more a feedback than a review, not ready for review.
Attachment #602086 - Flags: review?(mano) → feedback?(mano)
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.
Blocks: 730798
No longer depends on: 730798
Summary: Smart Bookmarks is broken → Markers in places popup views are broken and too much bugs-prone, causing broken queries
Attached patch patch v1.1 (obsolete) — Splinter Review
tested on win and mac and passes our b-c tests. I coulnd't ask more.
Attachment #602086 - Attachment is obsolete: true
Attachment #602128 - Flags: review?(mano)
Attachment #602086 - Flags: feedback?(mano)
Comment on attachment 602128 [details] [diff] [review]
patch v1.1

a couple bugs still, updating patch
Attachment #602128 - Attachment is obsolete: true
Attachment #602128 - Flags: review?(mano)
Attached patch patch v1.0 (obsolete) — Splinter Review
includes most of the stuff discussed on irc
Attachment #603330 - Flags: review?(mano)
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.
Attachment #603330 - Flags: review?(mano) → review+
Attached patch patch v1.1Splinter Review
Attachment #603330 - Attachment is obsolete: true
Duplicate of this bug: 730798
No longer blocks: 730798
https://hg.mozilla.org/mozilla-central/rev/5aa498cf8658
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
Keywords: addon-compat
Depends on: 734646
Depends on: 734653
Depends on: 753672
You need to log in before you can comment on or make changes to this bug.