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

RESOLVED FIXED in mozilla13

Status

()

Toolkit
Places
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: mak)

Tracking

({addon-compat, regression})

13 Branch
mozilla13
addon-compat, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Comment 1

5 years ago
@jrudkin also reports in mozillazine http://forums.mozillazine.org/viewtopic.php?p=11782043#p11782043
(Assignee)

Comment 2

5 years ago
thanks, I can reproduce
Assignee: nobody → mak77
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
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.
Attachment #602086 - Flags: review?(mano)
(Assignee)

Updated

5 years ago
Depends on: 730798
(Assignee)

Comment 4

5 years ago
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)
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 730798
No longer depends on: 730798
(Assignee)

Updated

5 years ago
Summary: Smart Bookmarks is broken → Markers in places popup views are broken and too much bugs-prone, causing broken queries
(Assignee)

Comment 6

5 years ago
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.
Attachment #602086 - Attachment is obsolete: true
Attachment #602128 - Flags: review?(mano)
Attachment #602086 - Flags: feedback?(mano)
(Assignee)

Comment 7

5 years ago
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)
(Assignee)

Comment 8

5 years ago
Created attachment 603330 [details] [diff] [review]
patch v1.0

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+
(Assignee)

Comment 10

5 years ago
Created attachment 603374 [details] [diff] [review]
patch v1.1
Attachment #603330 - Attachment is obsolete: true
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5aa498cf8658
Target Milestone: --- → mozilla13
(Assignee)

Updated

5 years ago
Duplicate of this bug: 730798
(Assignee)

Updated

5 years ago
No longer blocks: 730798
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/mozilla-central/rev/5aa498cf8658
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

5 years ago
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
(Reporter)

Updated

5 years ago
Depends on: 734646
(Reporter)

Updated

5 years ago
Depends on: 734653
(Reporter)

Updated

5 years ago
Depends on: 753672
You need to log in before you can comment on or make changes to this bug.