Bug 107363 – [EFL][WK2] Implement WebInspector::localizedStringsURL() on EFL

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Sudarsana Nagineni (babu)]

Implement WebInspector::localizedStringsURL() to return the URL of the localizedStrings.js.

[Sudarsana Nagineni (babu)]

Created attachment 183623 [details]
patch

[Chris Dumez]

Comment on attachment 183623 [details]
patch

Looks fine but you're not unskipping any test?

[Sudarsana Nagineni (babu)]

(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

[Chris Dumez]

(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 :)

[Gyuyoung Kim]

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 ?

[Sudarsana Nagineni (babu)]

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.

[Sudarsana Nagineni (babu)]

Created attachment 183626 [details]
patch

[Gyuyoung Kim]

Comment on attachment 183626 [details]
patch

LGTM.

[Chris Dumez]

This patch fixes about 300 failures on our build bots.

[Laszlo Gombos]

(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.

[Chris Dumez]

(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.

[Chris Dumez]

http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r140330%20(8310)/results.html

[Benjamin Poulain]

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.

[Sudarsana Nagineni (babu)]

Created attachment 183841 [details]
Patch

Take Benjamin's feedback into consideration.

[Sudarsana Nagineni (babu)]

Comment on attachment 183841 [details]
Patch

Thanks for review!

[WebKit Review Bot]

Comment on attachment 183841 [details]
Patch

Clearing flags on attachment: 183841

Committed r140372: <http://trac.webkit.org/changeset/140372>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.