WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
109061
<link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
https://bugs.webkit.org/show_bug.cgi?id=109061
Summary
<link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
Ruslan Abdikeev
Reported
2013-02-06 09:24:40 PST
No apple-touch-icons are fetched (and, hence, used) for CNN.con, workflowy.com etc. The same holds for apple-touch-icon-precomposed. Test steps: Load
http://aruslan.me/t1237621jsa/t.html
<html><head> <link href="
http://www.cnn.com/favicon.ie9.ico
" rel="Shortcut Icon" type="image/x-icon" /> <link href="
http://i.cdn.turner.com/cnn/.e/img/3.0/global/misc/apple-touch-icon.png
" rel="apple-touch-icon" type="image/png" /> <title>Title</title> </head><body>Body.</body></html> Observed results: 1. favicon.ie9.ico is fetched and shown correctly. 2. apple-touch-icon is NOT fetched; instead, the request is made to the website to fetch "default" fallback apple-touch-icon.png. Expected results: 1. favicon.ie9.ico is fetched and shown (no changes). 2. apple-touch-icon from CDN is fetched and shown.
Attachments
Patch
(1.43 KB, patch)
2013-02-06 09:40 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(4.99 KB, patch)
2013-02-06 11:47 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(7.11 KB, patch)
2013-02-06 14:55 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(14.23 KB, patch)
2013-02-06 18:42 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(16.32 KB, patch)
2013-02-11 17:55 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(16.18 KB, patch)
2013-02-11 18:36 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(18.60 KB, patch)
2013-02-13 10:03 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(18.45 KB, patch)
2013-02-13 20:02 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(18.44 KB, patch)
2013-02-19 18:08 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(18.50 KB, patch)
2013-02-22 14:48 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(22.44 KB, patch)
2013-02-26 12:32 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(22.07 KB, patch)
2013-03-01 08:50 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(22.14 KB, patch)
2013-03-01 18:48 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Patch
(22.11 KB, patch)
2013-03-04 10:09 PST
,
Ruslan Abdikeev
no flags
Details
Formatted Diff
Diff
Show Obsolete
(13)
View All
Add attachment
proposed patch, testcase, etc.
Ruslan Abdikeev
Comment 1
2013-02-06 09:40:36 PST
Created
attachment 186872
[details]
Patch
Ruslan Abdikeev
Comment 2
2013-02-06 11:47:18 PST
Created
attachment 186887
[details]
Patch
WebKit Review Bot
Comment 3
2013-02-06 12:51:01 PST
Comment on
attachment 186887
[details]
Patch
Attachment 186887
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/16388449
New failing tests: fast/dom/icon-url-list-apple-touch.html
Build Bot
Comment 4
2013-02-06 12:51:50 PST
Comment on
attachment 186887
[details]
Patch
Attachment 186887
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/16393364
Ruslan Abdikeev
Comment 5
2013-02-06 14:55:52 PST
Created
attachment 186926
[details]
Patch
Brady Eidson
Comment 6
2013-02-06 16:31:34 PST
Comment on
attachment 186926
[details]
Patch Why are we making a change that causes all desktop webkit clients to download a non-standard icon that was meant for mobile devices?
Ruslan Abdikeev
Comment 7
2013-02-06 16:36:31 PST
(In reply to
comment #6
)
> (From update of
attachment 186926
[details]
) > Why are we making a change that causes all desktop webkit clients to download a non-standard icon that was meant for mobile devices?
Desktop clients shouldn't define ENABLE(TOUCH_ICON_LOADING). When TOUCH_ICON_LOADING is not enabled, LinkRelAttribute ignores <link rel> for non-standard icons. As the result, Document.cpp doesn't have them anyway. This patch is to fix the error that causes all TOUCH_ICON_LOADING-enabled clients to never get touch icons.
Brady Eidson
Comment 8
2013-02-06 17:16:09 PST
Good point, but... There is a comment inside Document::ironURLs() that says: // Include any icons where type = link, rel = "shortcut icon". Admittedly this is an example of why comments aren't good enforcement over more obvious naming in code. But that point aside, this patch changes that behavior. This method is about "shortcut icons" that should be considered in the course of normal favicon loading whereas apple-touch-icons were always meant to be loaded out of band from normal page loading. We should rename iconURLs to shortcutIconURLs and leave it's behavior as-is. If you need to get at the apple-touch-icons for whatever reason, they can be in a different accessor. You could also refactor the current accessor to take an IconType parameter and leverage that for code reuse between the current behavior and the new behavior you hope to add.
Ruslan Abdikeev
Comment 9
2013-02-06 17:56:02 PST
(In reply to
comment #8
)
> We should rename iconURLs to shortcutIconURLs and leave it's behavior as-is. If you need to get at the apple-touch-icons for whatever reason, they can be in a different accessor. > You could also refactor the current accessor to take an IconType parameter and leverage that for code reuse between the current behavior and the new behavior you hope to add.
Agreed; I'm going to rename existing iconURLs() to shortcutIconURLs() keeping its semantics and add iconURLs(int iconTypes) that accepts a bitmask of IconTypes. Additionally, I'll extend Internals.idl, fix tests and surrounding call-sites. Are these names and parameters fine by you?
Brady Eidson
Comment 10
2013-02-06 18:09:53 PST
Sounds reasonable.
Ruslan Abdikeev
Comment 11
2013-02-06 18:42:59 PST
Created
attachment 186976
[details]
Patch
Ruslan Abdikeev
Comment 12
2013-02-06 18:45:26 PST
(In reply to
comment #10
)
> Sounds reasonable.
Please take a look, thanks!
Build Bot
Comment 13
2013-02-06 20:06:45 PST
Comment on
attachment 186976
[details]
Patch
Attachment 186976
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://queues.webkit.org/results/16417050
Build Bot
Comment 14
2013-02-06 22:08:29 PST
Comment on
attachment 186976
[details]
Patch
Attachment 186976
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/16409082
Build Bot
Comment 15
2013-02-06 23:05:24 PST
Comment on
attachment 186976
[details]
Patch
Attachment 186976
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/16414112
Build Bot
Comment 16
2013-02-06 23:10:10 PST
Comment on
attachment 186976
[details]
Patch
Attachment 186976
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/16403099
Brady Eidson
Comment 17
2013-02-07 08:57:31 PST
I'll hold off on review until ews is cleared up
Ruslan Abdikeev
Comment 18
2013-02-11 08:14:06 PST
(In reply to
comment #17
)
> I'll hold off on review until ews is cleared up
It fails to build on gtk/mac/mac-wk2/win with Undefined symbols for architecture x86_64: "__ZN7WebCore8Document8iconURLsEv" Strangely, this means that something refers to (now gone) iconURLs(void), but only at the link time. Otherwise it would have failed to compile the call to iconURLs(). Do we use precompiled libraries on gtk/mac/win? How could I trigger a clobber build or something?
Ruslan Abdikeev
Comment 19
2013-02-11 09:52:17 PST
gtk-ews fails with the reverse: ./.libs/libWebCoreInternals.a(./.libs/../Source/WebCore/testing/.libs/libWebCoreInternals_la-Internals.o):Internals.cpp:function WebCore::Internals::iconURLs(WebCore::Document*, int) const: error: undefined reference to 'WebCore::Document::iconURLs(int)' Looks like it uses correct headers, but links against a stale library.
Kent Tamura
Comment 20
2013-02-11 15:37:24 PST
(In reply to
comment #18
)
> (In reply to
comment #17
) > > I'll hold off on review until ews is cleared up > > It fails to build on gtk/mac/mac-wk2/win with > > Undefined symbols for architecture x86_64: > "__ZN7WebCore8Document8iconURLsEv"
WebCore in these ports export only specific symbols. See
http://trac.webkit.org/wiki/ExportingSymbols
.
Ruslan Abdikeev
Comment 21
2013-02-11 17:55:17 PST
Created
attachment 187737
[details]
Patch
Levi Weintraub
Comment 22
2013-02-11 18:04:15 PST
(In reply to
comment #21
)
> Created an attachment (id=187737) [details] > Patch
Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :(
Ruslan Abdikeev
Comment 23
2013-02-11 18:36:25 PST
Created
attachment 187746
[details]
Patch
Ruslan Abdikeev
Comment 24
2013-02-11 18:42:45 PST
(In reply to
comment #22
)
> (In reply to
comment #21
) > > Created an attachment (id=187737) [details] [details] > > Patch > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :(
Thanks, Levi, rebase FTW.
Levi Weintraub
Comment 25
2013-02-11 22:39:42 PST
(In reply to
comment #24
)
> (In reply to
comment #22
) > > (In reply to
comment #21
) > > > Created an attachment (id=187737) [details] [details] [details] > > > Patch > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > Thanks, Levi, rebase FTW.
Woo, green!
Ruslan Abdikeev
Comment 26
2013-02-12 15:15:19 PST
(In reply to
comment #25
)
> (In reply to
comment #24
) > > (In reply to
comment #22
) > > > (In reply to
comment #21
) > > > > Created an attachment (id=187737) [details] [details] [details] [details] > > > > Patch > > > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > > > Thanks, Levi, rebase FTW. > > Woo, green!
What are my next steps? On Mac it says "pass" and then it fails (?), on Mac-WK2 it fails as well.
Build Bot
Comment 27
2013-02-13 04:15:29 PST
Comment on
attachment 187746
[details]
Patch
Attachment 187746
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/16454806
Ruslan Abdikeev
Comment 28
2013-02-13 10:03:06 PST
Created
attachment 188107
[details]
Patch
Ruslan Abdikeev
Comment 29
2013-02-13 13:20:47 PST
Why I don't see Win in the EWS? Green is good. Green is healthy.
Ruslan Abdikeev
Comment 30
2013-02-13 20:02:48 PST
Created
attachment 188252
[details]
Patch
Ruslan Abdikeev
Comment 31
2013-02-13 20:04:21 PST
(In reply to
comment #25
)
> (In reply to
comment #24
) > > (In reply to
comment #22
) > > > (In reply to
comment #21
) > > > > Created an attachment (id=187737) [details] [details] [details] [details] > > > > Patch > > > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > > > Thanks, Levi, rebase FTW. > > Woo, green!
Rebased and re-submitted (the previous patch is green on all platforms, but for some reason EWS never put it to Win). Assuming Win is green this time, this should be a final version.
Ruslan Abdikeev
Comment 32
2013-02-19 18:08:39 PST
Created
attachment 189218
[details]
Patch
Levi Weintraub
Comment 33
2013-02-21 12:10:56 PST
Brady, would you take another look?
Ruslan Abdikeev
Comment 34
2013-02-22 14:48:16 PST
Created
attachment 189837
[details]
Patch
Ruslan Abdikeev
Comment 35
2013-02-22 14:51:26 PST
Brady -- would you take another look please? Rebase. The last 4 patches are exactly the same, but first the "win" EWS bot had unrelated problems (eventually became "green"), and now "cr-android" EWS bot is failing and "mac" build had some problems apparently.
Brady Eidson
Comment 36
2013-02-25 10:59:57 PST
Comment on
attachment 189837
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=189837&action=review
> Source/WebCore/dom/Document.cpp:4466 > +const Vector<IconURL>& Document::shortcutIconURLs() > +{ > + // Include any icons where type = link, rel = "shortcut icon". > + return iconURLs(Favicon); > +}
Where does the "Favicon" symbol come from and...
> Source/WebCore/dom/Document.cpp:4468 > +const Vector<IconURL>& Document::iconURLs(int iconTypes)
...why is this not an enum? This should be an enum
Ruslan Abdikeev
Comment 37
2013-02-25 11:26:45 PST
Comment on
attachment 189837
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=189837&action=review
>> Source/WebCore/dom/Document.cpp:4466 >> +} > > Where does the "Favicon" symbol come from and...
from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h).
>> Source/WebCore/dom/Document.cpp:4468 >> +const Vector<IconURL>& Document::iconURLs(int iconTypes) > > ...why is this not an enum? > > This should be an enum
It's a bitmask of requested icon types (this matches IconController::urlsForTypes(int iconTypes) API).
Ruslan Abdikeev
Comment 38
2013-02-26 09:46:18 PST
(In reply to
comment #36
)
> (From update of
attachment 189837
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=189837&action=review
> > > Source/WebCore/dom/Document.cpp:4466 > > +const Vector<IconURL>& Document::shortcutIconURLs() > > +{ > > + // Include any icons where type = link, rel = "shortcut icon". > > + return iconURLs(Favicon); > > +} > > Where does the "Favicon" symbol come from and... > > > Source/WebCore/dom/Document.cpp:4468 > > +const Vector<IconURL>& Document::iconURLs(int iconTypes) > > ...why is this not an enum? > > This should be an enum
Hi Brady, "Favicon" symbol comes from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h), and
>> +const Vector<IconURL>& Document::iconURLs(int iconTypes)
isn't enum because it's a bitmask of requested icon types. This matches IconController::urlsForTypes(int iconTypes) API. Please let me know if anything should be adjusted here. Otherwise, I'd appreciate if you could approve this patch. Yours, -- Ruslan.
Brady Eidson
Comment 39
2013-02-26 11:15:00 PST
(In reply to
comment #38
)
> (In reply to
comment #36
> > Hi Brady, > "Favicon" symbol comes from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h), and > >> +const Vector<IconURL>& Document::iconURLs(int iconTypes) > isn't enum because it's a bitmask of requested icon types. > This matches IconController::urlsForTypes(int iconTypes) API.
Then please call it iconTypesMask, and no harm in updating the old one also. r+ with that change.
Brady Eidson
Comment 40
2013-02-26 11:15:17 PST
Comment on
attachment 189837
[details]
Patch r+ with the question naming change.
Ruslan Abdikeev
Comment 41
2013-02-26 12:32:28 PST
Created
attachment 190340
[details]
Patch
WebKit Review Bot
Comment 42
2013-02-26 12:34:41 PST
Please wait for approval from
abarth@webkit.org
,
dglazkov@chromium.org
,
fishd@chromium.org
,
jamesr@chromium.org
or
tkent@chromium.org
before submitting, as this patch contains changes to the Chromium public API. See also
https://trac.webkit.org/wiki/ChromiumWebKitAPI
.
Ruslan Abdikeev
Comment 43
2013-02-26 12:36:56 PST
(In reply to
comment #40
)
> (From update of
attachment 189837
[details]
) > r+ with the question naming change.
Renamed iconTypes to iconTypesMask in Document, Internals, IconController and Chromium WebFrame.
Adam Barth
Comment 44
2013-02-26 14:50:01 PST
Comment on
attachment 190340
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=190340&action=review
> LayoutTests/fast/dom/icon-url-list-apple-touch.html:17 > + var supportsTouchIcons = window.internals.isTouchIconsLoadingSupported();
This is sort of unusual. Typically we just have different expected.txt files when compile-time options change rather than having the test automagically adapt.
Adam Barth
Comment 45
2013-02-26 15:11:22 PST
Comment on
attachment 190340
[details]
Patch API change LGTM.
Ruslan Abdikeev
Comment 46
2013-03-01 08:50:14 PST
Created
attachment 190974
[details]
Patch
Ruslan Abdikeev
Comment 47
2013-03-01 14:10:39 PST
(In reply to
comment #44
)
> (From update of
attachment 190340
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=190340&action=review
> > > LayoutTests/fast/dom/icon-url-list-apple-touch.html:17 > > + var supportsTouchIcons = window.internals.isTouchIconsLoadingSupported(); > > This is sort of unusual. Typically we just have different expected.txt files when compile-time options change rather than having the test automagically adapt.
Hi Adam and Brady, PTAL: I removed test auto-adaptation to apple-touch-icon loading and created two -expected -- one in the cross-platform with favicon only, and one in chromium with favicon and apple-touch-icon. Yours, -- Ruslan.
WebKit Review Bot
Comment 48
2013-03-01 15:25:01 PST
Comment on
attachment 190974
[details]
Patch
Attachment 190974
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://webkit-commit-queue.appspot.com/results/16877059
New failing tests: fast/dom/icon-url-list-apple-touch.html
WebKit Review Bot
Comment 49
2013-03-01 16:17:05 PST
Comment on
attachment 190974
[details]
Patch
Attachment 190974
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://webkit-commit-queue.appspot.com/results/16796239
New failing tests: fast/dom/icon-url-list-apple-touch.html
Ruslan Abdikeev
Comment 50
2013-03-01 18:48:48 PST
Created
attachment 191088
[details]
Patch
Ruslan Abdikeev
Comment 51
2013-03-01 18:54:28 PST
Moved the expectations from chromium/ to chromium-android/ due to chromium-linux failing because it doesn't ENABLE TOUCH_ICON_LOADING. I believe some of the chromium-mac targets could be enabling touch icons loading, in which case the expectations should be copied there from chromium-android. To summarize, I believe the auto-adapting test was a simpler design and it didn't require heavy maintenance.
Adam Barth
Comment 52
2013-03-02 23:32:51 PST
Comment on
attachment 191088
[details]
Patch This patch looks good. Thank you for taking the expected.txt route. I can understand why it seems like the adapting test might be easier when writing the test, but this approach is consistent with how we manage other platform differences and is easier to maintain in the long run.
WebKit Review Bot
Comment 53
2013-03-03 01:56:08 PST
Comment on
attachment 191088
[details]
Patch Clearing flags on attachment: 191088 Committed
r144567
: <
http://trac.webkit.org/changeset/144567
>
WebKit Review Bot
Comment 54
2013-03-03 01:56:16 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 55
2013-03-03 02:29:49 PST
Re-opened since this is blocked by
bug 111266
Ryosuke Niwa
Comment 56
2013-03-03 04:01:13 PST
(In reply to
comment #55
)
> Re-opened since this is blocked by
bug 111266
The patch caused Apple Windows build failures: e.g. 19>LINK : C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin\DumpRenderTree.dll not found or not built by the last incremental link; performing full link 19> Creating library C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.lib and object C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.exp 19>WebCoreTestSupport.lib(JSInternals.obj) : error LNK2019: unresolved external symbol "class JSC::JSValue __cdecl WebCore::toJS(class JSC::ExecState *,class WebCore::JSDOMGlobalObject *,class WebCore::CSSStyleDeclaration *)" (?toJS@WebCore@@YA?AVJSValue@JSC@@PAVExecState@
3@PAVJSDOMGlobalObject@1@PAVCSSStyleDeclaration@1@@Z
) referenced in function "__int64 __fastcall WebCore::jsInternalsPrototypeFunctionComputedStyleIncludingVisitedInfo(class JSC::ExecState *)" (?jsInternalsPrototypeFunctionComputedStyleIncludingVisitedInfo@WebCore@@YI_JPAVExecState@JSC@@@Z)
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio
It wasn’t a simple missing symbol error. It looked like something wasn’t compliling.
Ruslan Abdikeev
Comment 57
2013-03-04 08:28:45 PST
(In reply to
comment #56
)
> The patch caused Apple Windows build failures:
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio
> > It wasn’t a simple missing symbol error. It looked like something wasn’t compliling.
Thank you for jumping in on the issue! Given that the breakage wasn't caused by this patch (reverting this patch didn't help, and the build was fixed subsequently with
http://trac.webkit.org/changeset/144573
), I'm going to re-land it Objections?
Ruslan Abdikeev
Comment 58
2013-03-04 08:58:16 PST
(In reply to
comment #57
)
> (In reply to
comment #56
) > > The patch caused Apple Windows build failures: >
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio
> > > > It wasn’t a simple missing symbol error. It looked like something wasn’t compliling. > > Thank you for jumping in on the issue! > > Given that the breakage wasn't caused by this patch (reverting this patch didn't help, and the build was fixed subsequently with
http://trac.webkit.org/changeset/144573
), I'm going to re-land it > > Objections?
P.S. The revert (
http://trac.webkit.org/changeset/144572
) failed with the same link errors as this patch:
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63457
The real fix was submitted in a subsequent patch (
http://trac.webkit.org/changeset/144573
) and made the tree green:
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63458
That is, unless I'm reading it wrong.
Ruslan Abdikeev
Comment 59
2013-03-04 10:09:42 PST
Created
attachment 191267
[details]
Patch
Ruslan Abdikeev
Comment 60
2013-03-04 10:37:12 PST
(In reply to
comment #59
)
> Created an attachment (id=191267) [details] > Patch
This is a rebased patch. Previously the patch failed to compile on apple-win, because WebKitBuild\Debug\obj\WebCore\DerivedSources\JSInternals.cpp had to be regenerated, but there was an unrelated issue preventing this from happening. The build just before the revert successfully compiled everything related to this patch, but was blocked on the other issue:
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio
Reverting the patch didn't help, as the unrelated issue remained. Asking for permission to re-land the patch.
Adam Barth
Comment 61
2013-03-04 13:06:51 PST
> Asking for permission to re-land the patch.
The
http://build.webkit.org/waterfall?show=Apple%20Win%20Debug%20(Build
) bot is current not compiling. I'd prefer to re-land your patch when the bot is compiling so that we can verify that it does compile...
Ruslan Abdikeev
Comment 62
2013-03-04 13:08:02 PST
(In reply to
comment #61
)
> > Asking for permission to re-land the patch. > > The
http://build.webkit.org/waterfall?show=Apple%20Win%20Debug%20(Build
) bot is current not compiling. I'd prefer to re-land your patch when the bot is compiling so that we can verify that it does compile...
Sure thing. Thanks, Adam!
Adam Barth
Comment 63
2013-03-04 16:18:54 PST
Comment on
attachment 191267
[details]
Patch Windows is building again. Let's try landing this patch again.
WebKit Review Bot
Comment 64
2013-03-04 16:44:36 PST
Comment on
attachment 191267
[details]
Patch Clearing flags on attachment: 191267 Committed
r144696
: <
http://trac.webkit.org/changeset/144696
>
WebKit Review Bot
Comment 65
2013-03-04 16:44:44 PST
All reviewed patches have been landed. Closing bug.
Adam Barth
Comment 66
2013-03-04 17:17:39 PST
http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63524
confirms that this patch builds correct for apple-win.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug