Bug 100379

Summary: [Win] WebView didReceiveIcon delegate call doesn't have the icon in it
Product: WebKit Reporter: peavo
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, bfulgham, commit-queue, darin, peavo, roger_fong
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch as diff, output from svn-create-patch script.
none
Patch
none
Patch
none
Patch none

peavo
Reported 2012-10-25 07:27:20 PDT
Pass HBITMAP favicon to load delegate.
Attachments
Patch as diff, output from svn-create-patch script. (1.22 KB, patch)
2012-10-25 07:29 PDT, peavo 		no flags
DetailsFormatted DiffDiff
Patch (1.84 KB, patch)
2012-10-29 08:14 PDT, peavo 		no flags
DetailsFormatted DiffDiff
Patch (1.89 KB, patch)
2013-06-14 05:01 PDT, peavo 		no flags
DetailsFormatted DiffDiff
Patch (1.89 KB, patch)
2013-07-23 11:17 PDT, peavo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
peavo
Comment 1 2012-10-25 07:29:33 PDT
Created attachment 170645 [details] Patch as diff, output from svn-create-patch script.
Alexey Proskuryakov
Comment 2 2012-10-25 10:54:51 PDT
<rdar://problem/5491010>
Alexey Proskuryakov
Comment 3 2012-10-25 10:55:54 PDT
Would you be willing to submit a patch for inclusion in WebKit? Please follow the steps in <http://www.webkit.org/coding/contributing.html> - in particular, every patch needs a ChangeLog, and needs to be marked for review to be in review queue.
peavo
Comment 4 2012-10-26 10:39:05 PDT
Yes, will submit a patch soon.
peavo
Comment 5 2012-10-29 08:14:31 PDT
Created attachment 171249 [details] Patch
Alexey Proskuryakov
Comment 6 2012-11-05 09:53:36 PST
Comment on attachment 171249 [details] Patch Brady should probably review this.
peavo
Comment 7 2012-11-12 05:57:37 PST
Any takers? :)
Brent Fulgham
Comment 8 2013-06-13 10:16:31 PDT
Comment on attachment 171249 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171249&action=review Looks good. I think it would be simpler to create one IntSize and use it in both API calls that need it. Can you make that small change and I'll r+ this? > Source/WebKit/win/WebView.cpp:2842 > + sz.cy = 16; Why not just create the IntSize directly, then use it later? > Source/WebKit/win/WebView.cpp:2844 > + BitmapInfo bmInfo = BitmapInfo::create(IntSize(sz)); Use the IntSize here... > Source/WebKit/win/WebView.cpp:2848 > + Image* icon = iconDatabase().synchronousIconForPageURL(str, IntSize(sz)); ... and here!
peavo
Comment 9 2013-06-14 05:01:16 PDT
Created attachment 204703 [details] Patch
peavo
Comment 10 2013-06-14 05:02:48 PDT
Modifed patch after review.
peavo
Comment 11 2013-07-23 07:22:04 PDT
(In reply to comment #8) > Looks good. I think it would be simpler to create one IntSize and use it in both API calls that need it. Can you make that small change and I'll r+ this? r+? ;)
Brent Fulgham
Comment 12 2013-07-23 09:52:01 PDT
Comment on attachment 204703 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204703&action=review Sorry -- I didn't notice that we would have to cast that sz variable from IntSize to SIZE. Please correct that and I'll r+ it (today!) > Source/WebKit/win/WebView.cpp:2882 > + icon->getHBITMAPOfSize(hBitmap, &(SIZE)sz); This should be written as "&static_cast<SIZE>(sz)"
peavo
Comment 13 2013-07-23 11:17:24 PDT
Created attachment 207339 [details] Patch
Brent Fulgham
Comment 14 2013-07-23 11:19:48 PDT
r=me
peavo
Comment 15 2013-07-23 11:20:41 PDT
(In reply to comment #14) > r=me Thanks for reviewing!
WebKit Commit Bot
Comment 16 2013-07-23 12:00:51 PDT
Comment on attachment 207339 [details] Patch Clearing flags on attachment: 207339 Committed r153059: <http://trac.webkit.org/changeset/153059>
WebKit Commit Bot
Comment 17 2013-07-23 12:00:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.