Closed Bug 730752 Opened 13 years ago Closed 12 years ago

Replace all old synchronous favicons calls in Suite with Asynchronous Favicons API.

Categories

(SeaMonkey :: Bookmarks & History, defect)

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(seamonkey2.10 wontfix, seamonkey2.11 fixed)

Status:
RESOLVED FIXED
Milestone:
seamonkey2.12
Tracking Flags:
Tracking Status
seamonkey2.10 --- wontfix
seamonkey2.11 --- fixed

People

(Reporter: philip.chee, Assigned: sgautherie)

References

Details

Attachments

(2 files)

(Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6]
(deleted), patch
iannbugzilla
: review+
iannbugzilla
: approval-comm-aurora+
Details | Diff | Splinter Review
(Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comments 11 and 12]
(deleted), patch
iannbugzilla
: review+
iannbugzilla
: approval-comm-aurora+
Details | Diff | Splinter Review
References: Bug 713642 - (asyncFaviconCallers) Replace all old synchronous favicons calls in the codebase. Bug 713269 - Use Asynchronous Favicons API for PlacesUtils.jsm. Bug 728141 - Replace old synchronous favicons calls in browser.
Depends on: 730849, SM-livemarksIO
Summary: Replace all old synchronous favicons calls in the codebase with Asynchronous Favicons API. → Replace all old synchronous favicons calls in Suite with Asynchronous Favicons API.
(In reply to Philip Chee from comment #0) > Bug 713269 - Use Asynchronous Favicons API for PlacesUtils.jsm. Ftr, this is Toolkit: nothing to port.
Depends on: asyncFaviconCallers, 728141
Blocks: FF2SM
Target Milestone: --- → seamonkey2.10
status-seamonkey2.10: --- → affected
Target Milestone: seamonkey2.10 → seamonkey2.11
Depends on: 728174
status-seamonkey2.11: --- → affected
Target Milestone: seamonkey2.11 → seamonkey2.12
Depends on: 750855
Succeeded (with a workaround) as https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=dc5c1f8f91d4 [Approval Request Comment] No risk, test-only.
Attachment #624364 - Flags: review?(iann_bugzilla)
Attachment #624364 - Flags: approval-comm-aurora?
Comment on attachment 624364 [details] [diff] [review] (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6] This is a trivial code copy, except for TEST_FAVICON_DATA_SIZE value.
Comment on attachment 624364 [details] [diff] [review] (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6] Land this on trunk and once you're happy tests are fine, will consider the approval request.
Attachment #624364 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 624364 [details] [diff] [review] (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6] https://hg.mozilla.org/comm-central/rev/c92117e25748
Attachment #624364 - Attachment description: (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey → (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comment 5]
Attachment #624364 - Flags: approval-comm-aurora? → approval-comm-aurora+
No longer blocks: FF2SM
No longer depends on: asyncFaviconCallers, SM-livemarksIO, 728141, 728174, 730849, 750855
Keywords: checkin-needed
Whiteboard: [c-n: c92117e25748 to c-a]
Comment on attachment 624364 [details] [diff] [review] (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6] http://hg.mozilla.org/releases/comm-aurora/rev/abd0b7f43287
Attachment #624364 - Attachment description: (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comment 5] → (Av1) Port |Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import| to SeaMonkey [Checked in: Comments 5 and 6]
Keywords: checkin-needed
Whiteboard: [c-n: c92117e25748 to c-a]
I wonder whether this is it or if being async breaks these code. (Untested)
Attachment #629558 - Flags: review?(iann_bugzilla)
(In reply to Serge Gautherie (:sgautherie) from comment #7) > I wonder whether this is it or if being async breaks these code. { (In reply to Paolo Amadini [:paolo] from bug 728141 comment #1) > The second change is just changing a function name with an alias. } answers this: trivial change only ;-) > (Untested)
Assignee: nobody → sgautherie.bz
No longer blocks: FF2SM
Status: NEW → ASSIGNED
status-seamonkey2.10: affected → wontfix
Flags: in-testsuite+
There might be one test that needs fixing: http://mxr.mozilla.org/comm-central/source/suite/common/places/tests/unit/test_bookmarks_html.js#450 I believe you will have to use PlacesUtils.favicons.getFaviconURLForPage instead but Firefox haven't fixed their equivalent test yet, so maybe is not needed/been forgotten/still to be done.
(In reply to Ian Neal from comment #9) > Firefox haven't fixed their equivalent test yet, so maybe is not > needed/been forgotten/still to be done. Iiuc, that is bug 740468.
Blocks: asyncFaviconCallers
No longer depends on: asyncFaviconCallers
Attachment #629558 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 629558 [details] [diff] [review] (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comments 11 and 12] http://hg.mozilla.org/comm-central/rev/6417fa1a06e1 [Approval Request Comment] No risk, trivial aliasing. (Too late for SM 2.10.)
Attachment #629558 - Attachment description: (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey → (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comment 11]
Attachment #629558 - Flags: approval-comm-aurora?
Attachment #629558 - Flags: approval-comm-aurora? → approval-comm-aurora+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: 6417fa1a06e1 to c-a]
Comment on attachment 629558 [details] [diff] [review] (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comments 11 and 12] http://hg.mozilla.org/releases/comm-beta/rev/d64fc962b931 (c-b is the new c-a...)
Attachment #629558 - Attachment description: (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comment 11] → (Bv1) Port |Bug 728141 - Replace old synchronous favicons calls in browser| to SeaMonkey [Checked in: Comments 11 and 12]
Keywords: checkin-needed
Whiteboard: [c-n: 6417fa1a06e1 to c-a]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: