Closed Bug 960660 Opened 7 years ago Closed 7 years ago
Add icons to places rocketbar results
No description provided.
There was one change that got intertwined in this commit and its a bit annoying to unwind, so I left it in, Datastore does have change events, so we now use them instead of manually triggering changes via the rocketbar interapp api, this is far more reliable Apart from that its a fairly simple change, currently as with the rest of the history we just maintain a list of currently active urls + icons, these will have to be persisted however doing it this way gives us freedom to experiment with the implementation without breaking things going forward. There is a test added along with this, it currently intermittently fails, very likely due to the same issue that has the other tests intermittently fail so I have added it as a skip and currently working on enabling them (in https://bugzilla.mozilla.org/show_bug.cgi?id=961137)
Attachment #8362929 - Flags: review?(kgrandon)
It looks like this is a clean marge against the rocketbar branch and is working
Comment on attachment 8362929 [details] [review] https://github.com/mozilla-b2g/gaia/pull/15560 Dale - this looks good. I think we mainly just need to move the location of the test file. I was also having problems actually getting icons to load in my search app. I'm seeing a few icons, but there's a bunch of sites that probably should have icons that don't (cnn.com) for example.
Attachment #8362929 - Flags: review?(kgrandon) → review+
I wasnt quite sure what you were aiming for with the generic class names so I landed as is, it seems the places results arent gonna be particularly reusable styles, but as said can fix as a follow up. I think I have seen issues with favicons not being set as well, the test added was a lot more flakey than the others, but working to reenable the tests now
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
Whiteboard: [systemsfe] → [systemsfe], [p=8]
You need to log in before you can comment on or make changes to this bug.