Summary: | [EFL][WK2] Implement WebInspector::localizedStringsURL() on EFL | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sudarsana Nagineni (babu) <naginenis> | ||||||||
Component: | WebKit EFL | Assignee: | Sudarsana Nagineni (babu) <naginenis> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | andersca, benjamin, cdumez, gyuyoung.kim, kenneth, kling, laszlo.gombos, lucas.de.marchi, mitz, mjs, rakuco, sam, tmpsantos, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Sudarsana Nagineni (babu)
2013-01-18 23:52:56 PST
Created attachment 183623 [details]
patch
Comment on attachment 183623 [details]
patch
Looks fine but you're not unskipping any test?
(In reply to comment #2) > (From update of attachment 183623 [details]) > Looks fine but you're not unskipping any test? Thanks Chris. Tests were unskipped already. We have 321 Web inspector tests failing on Debug bot now after r140044. http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r140060%20%288198%29/results.html http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/8198/steps/layout-test/logs/stdio (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 183623 [details] [details]) > > Looks fine but you're not unskipping any test? > > Thanks Chris. > > Tests were unskipped already. We have 321 Web inspector tests failing on Debug bot now after r140044. > > http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r140060%20%288198%29/results.html > > http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/8198/steps/layout-test/logs/stdio Oh right. All good then :) Comment on attachment 183623 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=183623&action=review > Source/WebKit2/WebProcess/WebPage/efl/WebInspectorEfl.cpp:33 > #include <wtf/text/WTFString.h> It looks we don't need to include WTFString.h anymore, isn't it ? Comment on attachment 183623 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=183623&action=review >> Source/WebKit2/WebProcess/WebPage/efl/WebInspectorEfl.cpp:33 >> #include <wtf/text/WTFString.h> > > It looks we don't need to include WTFString.h anymore, isn't it ? I will check and likely remove. Created attachment 183626 [details]
patch
Comment on attachment 183626 [details]
patch
LGTM.
This patch fixes about 300 failures on our build bots. (In reply to comment #9) > This patch fixes about 300 failures on our build bots. I was confused by this so just to clarify - these failing tests are not skipped at the moment that's why the patch does not include a change in the Skipped file. This patch will bring back the bot to green. Patch looks good to me. (In reply to comment #10) > (In reply to comment #9) > > > This patch fixes about 300 failures on our build bots. > > I was confused by this so just to clarify - these failing tests are not skipped at the moment that's why the patch does not include a change in the Skipped file. This patch will bring back the bot to green. > > Patch looks good to me. This is correct. Those tests are not skipped and failing on the WK2 EFL debug build bot. Comment on attachment 183626 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=183626&action=review > Source/WebKit2/WebProcess/WebPage/efl/WebInspectorEfl.cpp:43 > + StringBuilder builder; > + builder.appendLiteral("file://"); > + builder.append(WebCore::inspectorResourcePath()); > + builder.appendLiteral("/localizedStrings.js"); > + > + return builder.toString(); This should be using the String Operators, not String builder. Created attachment 183841 [details]
Patch
Take Benjamin's feedback into consideration.
Comment on attachment 183841 [details]
Patch
Thanks for review!
Comment on attachment 183841 [details] Patch Clearing flags on attachment: 183841 Committed r140372: <http://trac.webkit.org/changeset/140372> All reviewed patches have been landed. Closing bug. |