Bug 119559 – [EFL] Web Inspector: Move to new web inspector

• Component: Web Inspector
• Status: RESOLVED FIXED

[Marcelo Morais]

Replace the old inspector to the new web inspector on EFL port.

[Radar WebKit Bug Importer]

<rdar://problem/14678353>

[Marcelo Morais]

Created attachment 209113 [details]
Screenshot from first version

[Marcelo Morais]

After hooking up some of the bits needed, I am able to launch a functional version of the new Inspector in EFL's MiniBrowser (see attachedment #209113).
  
As one might notice, some of the SVG icons at the top are not properly displayed. Have you guys faced similar issues or have an idea on where to start debugging it from?

[Joseph Pecoraro]

Heh, that is pretty wild.

For starters, you may want to give #toolbar a background gradient / color instead of white. Probably somewhat like the old inspector did this. I think it checked InspectorFrontendHost.platform and added a style class to the body, and used that in CSS selectors for setting a toolbar background.

As for why the embossing doesn't work, I don't know. Try bumping _imageStorageFormatVersion in ImageUtilities.js to see if we're still reusing the generated images from before switching to SVG images (generated images are stored in a WebSQL Database). If that doesn't work, then you will need to dig into ImageUtilities generateEmbossedImages.

[Marcelo Morais]

Created attachment 209677 [details]
New screenshot

Hello,

After a week working with Andre Loureiro in this bug we didn't find any reason to WebInspector looks that weird as shown in the first picture.

We debug ImageUtilities, changed some css files and tried another things but the inspector wasn't getting any better.

Today i did a rebase with webkit trunk and the inspector looks good without any changes, so something wasn't good in the hash that we were using but is fixed now.

I think that the presentation is good now, but still have some work to do and we have to test it completly.

I hope we have the patch later this week.

[Joseph Pecoraro]

Ahh, this may have been when only some of the images were converted to SVGs but not all of them where yet. Excellent, looking forward to a patch!

[Marcelo Morais]

Created attachment 210111 [details]
Patch

Hi,

Here is the patch.

The new inspector is working fine with MiniBrowser and with EWebLauncher.

The known issues are:

1 - Like the old inspector, the dock controls are not working.
2 - Context menu are not working, MiniBrowser and EWebLauncher needs some work to enable it, in the old inspector also have problems like that, e.g open the menu on elements tab using the mouse's right click.
3 - The frame's combo box in content browser area is not working.

But this are minor issues and do not affect the main working of the inspector.

Maybe a good idea is to open bugs to improve these browsers and fix these issues. 

We don't know about layout tests for this new WebInspector, the old ones aren't skipped in TestExpectations.

Are there any plans to these new Layout Tests for the new inspector?

[Timothy Hatcher]

Comment on attachment 210111 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210111&action=review

Otherwise looks good to me.

> Source/WebKit2/ChangeLog:2
> [phone]  Marcelo Morais  <[e-mail]>
> [phone]  Andre Loureiro  <[e-mail]>

I don't think this is typical. Usually you mention other people in the description and only supply one ChangeLog header. Otherwise this might confuse some scripts we have.

[Joseph Pecoraro]

(In reply to comment #7)
> Created an attachment (id=210111) [details]
> The new inspector is working fine with MiniBrowser and with EWebLauncher.

Excellent!

> The known issues are:
> 
> 1 - Like the old inspector, the dock controls are not working.
> 2 - Context menu are not working, MiniBrowser and EWebLauncher needs some work to enable it, in the old inspector also have problems like that, e.g open the menu on elements tab using the mouse's right click.
> 3 - The frame's combo box in content browser area is not working.
> 
> But this are minor issues and do not affect the main working of the inspector.

How about fonts and keyboard shortcuts? I suspect the font situation could be improved and keyboard shortcuts should likely be using Ctrl instead of Meta right now (most of the KeyboardShortcuts right now are tuned to OS X with ⌘ (Command) key.


> Maybe a good idea is to open bugs to improve these browsers and fix these issues. 

Yes, that is the ideal path to getting these fixed. Quick link to create a Web Inspector bug is:
<http://webkit.org/new-inspector-bug>


> We don't know about layout tests for this new WebInspector, the old ones aren't skipped in TestExpectations.
> 
> Are there any plans to these new Layout Tests for the new inspector?

Yep you're correct. Instead of testing the UserInterface we're putting I've been focus on testing the protocol (LayoutTest/inspector-protocol). The existing LayoutTests/inspector tests are pretty tightly coupled with the old inspector frontend. That makes changing the frontend more difficult and time consuming because you also have to update the tests that relied on private functions / interfaces in the frontend.

[Marcelo Morais]

Created attachment 210153 [details]
Fixed Changelog

Thanks for the comments :)

Fixed Changelog now.

About Joseph's comments, we didn't touch in fonts and keyboard shortcuts.

I talked with Andre Loureiro about that and we saw that patch for enable the new inspector for Windows landed recently (https://bugs.webkit.org/show_bug.cgi?id=120098) didn't touch with this as well, maybe we need create  
another bug for these general changes, don't you think?

[Joseph Pecoraro]

(In reply to comment #10)
> Created an attachment (id=210153) [details]
> … maybe we need create another bug for these general changes, don't you think?

Yep, I agree. That was what I was suggesting as well.

[Joseph Pecoraro]

Comment on attachment 210153 [details]
Fixed Changelog

View in context: https://bugs.webkit.org/attachment.cgi?id=210153&action=review

> Source/PlatformEfl.cmake:-37
> -    find_program(UGLIFYJS_EXECUTABLE uglifyjs)
> -    if (UGLIFYJS_EXECUTABLE AND (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Debug"))
> -        file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
> -        set(all_js_files
> -            ${frontend_js_files}
> -            "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
> -            "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
> -        )
> -
> -        foreach (js_file ${all_js_files})
> -            get_filename_component(filename ${js_file} NAME)
> -            install(CODE
> -                "execute_process(
> -                    COMMAND ${UGLIFYJS_EXECUTABLE} --overwrite ${filename}
> -                    WORKING_DIRECTORY ${CMAKE_INSTALL_PREFIX}/${WEB_INSPECTOR_DIR})")
> -        endforeach ()
> -    endif ()

It looks like this used to combine and minify the JS files for the old frontend. You may want to do something like this for the new frontend. The code would likely be very similar.

[Andre Loureiro (IRC: loureiro)]

(In reply to comment #12)
> (From update of attachment 210153 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210153&action=review
> 
> > Source/PlatformEfl.cmake:-37
> > -    find_program(UGLIFYJS_EXECUTABLE uglifyjs)
> > -    if (UGLIFYJS_EXECUTABLE AND (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Debug"))
> > -        file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
> > -        set(all_js_files
> > -            ${frontend_js_files}
> > -            "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
> > -            "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
> > -        )
> > -
> > -        foreach (js_file ${all_js_files})
> > -            get_filename_component(filename ${js_file} NAME)
> > -            install(CODE
> > -                "execute_process(
> > -                    COMMAND ${UGLIFYJS_EXECUTABLE} --overwrite ${filename}
> > -                    WORKING_DIRECTORY ${CMAKE_INSTALL_PREFIX}/${WEB_INSPECTOR_DIR})")
> > -        endforeach ()
> > -    endif ()
> 
> It looks like this used to combine and minify the JS files for the old frontend. You may want to do something like this for the new frontend. The code would likely be very similar.
Do you mean execute WebInspectorUI/Scripts/jsmin.py and cssmin.py?

[Joseph Pecoraro]

> Do you mean execute WebInspectorUI/Scripts/jsmin.py and cssmin.py?

Yes, either that or sticking with uglifyjs like you have now for minifying the JavaScript.

[Marcelo Morais]

Created attachment 210160 [details]
Fixed patch

Fixes suggested by Joseph.

[Joseph Pecoraro]

Comment on attachment 210160 [details]
Fixed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210160&action=review

> Source/PlatformEfl.cmake:25
> -        file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
> +        file(GLOB frontend_js_files "${PROJECT_SOURCE_DIR}/Source/WebInspectorUI/UserInterface/*.js")
>          set(all_js_files
>              ${frontend_js_files}
> -            "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
> -            "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
> +            "${PROJECT_SOURCE_DIR}/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js"
>          )

This uglifyjs command below does "--overwrite". So it sounds like you would want to only update the ${WEB_INSPECTOR_DIR} files, not the actual files in Source/WebInspectorUI. Especially if you have already copied the files over. The old code had the same problem, so I'm not sure what this part of code is doing.

I would expect to see something like:

    file(GLOB frontend_js_files "${WEB_INSPECTOR_DIR}/*.js")
    file(GLOB codemirror_js_files "${WEB_INSPECTOR_DIR}/External/CodeMirror/*.js")
    set(all_js_files
        ${frontend_js_files}
        ${codemirror_js_files}
    )

Does that make sense?

[Seokju Kwon]

> Source/PlatformEfl.cmake:6
> +        COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/Source/WebInspectorUI/UserInterface ${WEB_INSPECTOR_DIR}

How about using variable for ${PROJECT_SOURCE_DIR}/Source/WebInspectorUI ?

in CMakeLists.txt
+set(WEBINSPECTORUI_DIR "${CMAKE_SOURCE_DIR}/Source/WebInspectorUI")

> Source/PlatformEfl.cmake:7
> +        COMMAND ${CMAKE_COMMAND} -E copy ${PROJECT_SOURCE_DIR}/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js ${WEB_INSPECTOR_DIR}

Ditto.

> Source/PlatformEfl.cmake:24
> +            "${PROJECT_SOURCE_DIR}/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js"

Ditto.

And you need to add new filetype(SVG) for FILES_MATCHING of install, or remove it.

[Andre Loureiro (IRC: loureiro)]

> I would expect to see something like:
> 
>     file(GLOB frontend_js_files "${WEB_INSPECTOR_DIR}/*.js")
>     file(GLOB codemirror_js_files "${WEB_INSPECTOR_DIR}/External/CodeMirror/*.js")
>     set(all_js_files
>         ${frontend_js_files}
>         ${codemirror_js_files}
>     )
> 
> Does that make sense?

Hi, I'm not sure about it, so I've looked at WebInspectorUI/Scripts and found some python and shell scripts there, so there's one that I saw called copy-user-interface-resources.sh, so I think if we try to execute this one at build time it could resolve all these related issues about minify the .js and .css code, if I am wrong, please clarify these scripts.

Thanks.

[Marcelo Morais]

Created attachment 210310 [details]
patch

Sorry for the long delay, I was testing the uglifyjs.

In this new patch, i added the variable according to Seokju Kwon suggested.

About the uglifyjs, the code is correct, the code minimization will happen when installing webkit.

You can see the minimized code in /usr/local/share/ewebkit-0/inspector (this is the default installation path).

[Seokju Kwon]

Comment on attachment 210310 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210310&action=review

> Source/cmake/OptionsEfl.cmake:237
> +    set(WEBINSPECTORUI_DIR "${PROJECT_SOURCE_DIR}/Source/WebInspectorUI")

Use CMAKE_SOURCE_DIR instead of PROJECT_SOURCE_DIR for consistency?

[Marcelo Morais]

Created attachment 210365 [details]
patch

Hi Seokju, thanks for the review, the patch is fixed with your suggestion now.

[Build Bot]

Comment on attachment 210365 [details]
patch

Attachment 210365 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1687331

New failing tests:
fast/workers/termination-with-port-messages.html

[Build Bot]

Created attachment 210371 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4

[Gyuyoung Kim]

BTW, can't we adjust this into EFL WK2 as well ?

[Marcelo Morais]

(In reply to comment #24)
> BTW, can't we adjust this into EFL WK2 as well ?

Hello Gyuyoung Kim, could you explain better to me? 

Because this is already working on EFL WK2 using MiniBrowser.

Thanks!

[Gyuyoung Kim]

(In reply to comment #25)
> (In reply to comment #24)
> > BTW, can't we adjust this into EFL WK2 as well ?
> 
> Hello Gyuyoung Kim, could you explain better to me? 
> 
> Because this is already working on EFL WK2 using MiniBrowser.

Aha, I see. I didn't check it.

[Marcelo Morais]

> Aha, I see. I didn't check it.

:) Any comments or do you think its good now?

[Gyuyoung Kim]

Comment on attachment 210365 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210365&action=review

R- because of wrong ChangeLog.

> Source/WebKit2/ChangeLog:11
> +        * WebCoreSupport/InspectorClientEfl.cpp:

This patch doesn't touch WebCoreSupport/InspectorClientEfl.cpp.

> Source/WebKit2/ChangeLog:14
> +        (WebKit::WebInspectorProxy::inspectorPageURL):

Please mention what is changed here.

> Source/WebKit2/ChangeLog:15
> +        * Source/PlatformEfl.cmake:

I wonder why this file is mentioned in WebKit2/ChangeLog. Don't you generate ChangeLog using Prepare-ChangeLog script ?

[Marcelo Morais]

Created attachment 210461 [details]
Fixed Changelog

Fixed Changelog.

[Gyuyoung Kim]

Comment on attachment 210461 [details]
Fixed Changelog

View in context: https://bugs.webkit.org/attachment.cgi?id=210461&action=review

> Source/PlatformEfl.cmake:-9
> -        COMMAND ${CMAKE_COMMAND} -E copy ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js ${WEB_INSPECTOR_DIR}

Don't you need to install these files as well ? 

    if (UGLIFYJS_EXECUTABLE AND (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Debug"))
        file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
        set(all_js_files
            ${frontend_js_files}
            "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
            "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
        )

        foreach (js_file ${all_js_files})
            get_filename_component(filename ${js_file} NAME)
            install(CODE
                "execute_process(
                    COMMAND ${UGLIFYJS_EXECUTABLE} --overwrite ${filename}
                    WORKING_DIRECTORY ${CMAKE_INSTALL_PREFIX}/${WEB_INSPECTOR_DIR})")
        endforeach ()
    endif ()

[Seokju Kwon]

(In reply to comment #30)
> (From update of attachment 210461 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210461&action=review
> 
> > Source/PlatformEfl.cmake:-9
> > -        COMMAND ${CMAKE_COMMAND} -E copy ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js ${WEB_INSPECTOR_DIR}
> 
> Don't you need to install these files as well ? 
> 
>     if (UGLIFYJS_EXECUTABLE AND (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Debug"))
>         file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
>         set(all_js_files
>             ${frontend_js_files}
>             "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
>             "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
>         )
> 
>         foreach (js_file ${all_js_files})
>             get_filename_component(filename ${js_file} NAME)
>             install(CODE
>                 "execute_process(
>                     COMMAND ${UGLIFYJS_EXECUTABLE} --overwrite ${filename}
>                     WORKING_DIRECTORY ${CMAKE_INSTALL_PREFIX}/${WEB_INSPECTOR_DIR})")
>         endforeach ()
>     endif ()

Use in WebInspectorUI/UserInterface/InspectorBackendCommands.js
in New Web Inspector.

[Seokju Kwon]

(In reply to comment #31)
> (In reply to comment #30)
> > (From update of attachment 210461 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=210461&action=review
> > 
> > > Source/PlatformEfl.cmake:-9
> > > -        COMMAND ${CMAKE_COMMAND} -E copy ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js ${WEB_INSPECTOR_DIR}
> > 
> > Don't you need to install these files as well ? 
> > 
> >     if (UGLIFYJS_EXECUTABLE AND (NOT ${CMAKE_BUILD_TYPE} STREQUAL "Debug"))
> >         file(GLOB frontend_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
> >         set(all_js_files
> >             ${frontend_js_files}
> >             "${WEBCORE_DIR}/English.lproj/localizedStrings.js"
> >             "${DERIVED_SOURCES_WEBCORE_DIR}/InspectorBackendCommands.js"
> >         )
> > 
> >         foreach (js_file ${all_js_files})
> >             get_filename_component(filename ${js_file} NAME)
> >             install(CODE
> >                 "execute_process(
> >                     COMMAND ${UGLIFYJS_EXECUTABLE} --overwrite ${filename}
> >                     WORKING_DIRECTORY ${CMAKE_INSTALL_PREFIX}/${WEB_INSPECTOR_DIR})")
> >         endforeach ()
> >     endif ()
> 
> Use in WebInspectorUI/UserInterface/InspectorBackendCommands.js
> in New Web Inspector.

It was a wrong operation by mistake. :-(
New Web Inspector uses WebInspectorUI/UserInterface/InspectorBackendCommands.js.
So we dont need it.

[Marcelo Morais]

Hi, thanks for reviewing, all files from old inspector are no longer necessary and now i am handling only the necessary files of the new web inspector and all of them are being installed correctly.

Do you guys think that we need some fix or is the patch good already?

[Gyuyoung Kim]

(In reply to comment #33)
> Hi, thanks for reviewing, all files from old inspector are no longer necessary and now i am handling only the necessary files of the new web inspector and all of them are being installed correctly.
> 
> Do you guys think that we need some fix or is the patch good already?

My stance is that we don't need to keep unused files or code.

[Marcelo Morais]

(In reply to comment #34)
> (In reply to comment #33)
> > Hi, thanks for reviewing, all files from old inspector are no longer necessary and now i am handling only the necessary files of the new web inspector and all of them are being installed correctly.
> > 
> > Do you guys think that we need some fix or is the patch good already?
> 
> My stance is that we don't need to keep unused files or code.

Yes, that's why i removed all references to old inspector. Now we are using only the code from new inspector. 

Sorry if i misunderstood, but can you please explain to me what is the problem now? :)

[Gyuyoung Kim]

Comment on attachment 210461 [details]
Fixed Changelog

Looks ok.

[Gyuyoung Kim]

(In reply to comment #35)
> (In reply to comment #34)
> > (In reply to comment #33)
> > > Hi, thanks for reviewing, all files from old inspector are no longer necessary and now i am handling only the necessary files of the new web inspector and all of them are being installed correctly.
> > > 
> > > Do you guys think that we need some fix or is the patch good already?
> > 
> > My stance is that we don't need to keep unused files or code.
> 
> Yes, that's why i removed all references to old inspector. Now we are using only the code from new inspector. 
> 
> Sorry if i misunderstood, but can you please explain to me what is the problem now? :)

ok, looks nice. But, it would be good if seokju have a final review before landing.

[Marcelo Morais]

(In reply to comment #37)
> (In reply to comment #35)
> > (In reply to comment #34)
> > > (In reply to comment #33)
> > > > Hi, thanks for reviewing, all files from old inspector are no longer necessary and now i am handling only the necessary files of the new web inspector and all of them are being installed correctly.
> > > > 
> > > > Do you guys think that we need some fix or is the patch good already?
> > > 
> > > My stance is that we don't need to keep unused files or code.
> > 
> > Yes, that's why i removed all references to old inspector. Now we are using only the code from new inspector. 
> > 
> > Sorry if i misunderstood, but can you please explain to me what is the problem now? :)
> 
> ok, looks nice. But, it would be good if seokju have a final review before landing.

Thank you Gyuyoung, if Seokju find some problem I'll be glad to fix it.

[Seokju Kwon]

I wonder that why you don't add 'inspectorPageIndex.html' for Remote web inspector. It seems that Remote web inspector works properly (without Bug 116587).
And you could remove some tests related to inspector from TestExpectation.

[Seokju Kwon]

(In reply to comment #39)
> I wonder that why you don't add 'inspectorPageIndex.html' for Remote web inspector. It seems that Remote web inspector works properly (without Bug 116587).
> And you could remove some tests related to inspector from TestExpectation.

The new inspector UI does not support the current inspector testing harness.
So should we have all-inspector-tests skipped ?

[Gyuyoung Kim]

(In reply to comment #40)
> (In reply to comment #39)
> > I wonder that why you don't add 'inspectorPageIndex.html' for Remote web inspector. It seems that Remote web inspector works properly (without Bug 116587).
> > And you could remove some tests related to inspector from TestExpectation.
> 
> The new inspector UI does not support the current inspector testing harness.
> So should we have all-inspector-tests skipped ?

If this support for new inspector is not urgent, I prefer to support all inspector tests as well. Marcelo, did you check all inspector tests ? I clear r+ until verifying all inspector tests.

[Marcelo Morais]

(In reply to comment #41)
> (In reply to comment #40)
> > (In reply to comment #39)
> > > I wonder that why you don't add 'inspectorPageIndex.html' for Remote web inspector. It seems that Remote web inspector works properly (without Bug 116587).
> > > And you could remove some tests related to inspector from TestExpectation.
> > 
> > The new inspector UI does not support the current inspector testing harness.
> > So should we have all-inspector-tests skipped ?
> 
> If this support for new inspector is not urgent, I prefer to support all inspector tests as well. Marcelo, did you check all inspector tests ? I clear r+ until verifying all inspector tests.

Hi Seokju and Gyuyoung, yes I checked the tests and they are not supported for this new WebInspector. As Seokju already asked about it in the master bug (https://bugs.webkit.org/show_bug.cgi?id=118676) and as you can see in comment #7 the tests aren't skipped for now.

The Windows port, already landed, didn't touched the tests (https://bugs.webkit.org/show_bug.cgi?id=120098), but GTK port, still under review, are skipping the tests (https://bugs.webkit.org/show_bug.cgi?id=120647).

So I plan to touch the tests when a new test harness is available like all other platforms should do. What do you guys think? It's ok to do that or the correct is skip the tests expectations for now?

[Joseph Pecoraro]

See my comments about tests in Comment 9.

[Gyuyoung Kim]

(In reply to comment #43)
> See my comments about tests in Comment 9.

If we can't test existing inspector tests based on new inspector, I think we need to skip the tests for EFL port.

[Marcelo Morais]

(In reply to comment #44)
> (In reply to comment #43)
> > See my comments about tests in Comment 9.
> 
> If we can't test existing inspector tests based on new inspector, I think we need to skip the tests for EFL port.

Ok then, I'll skip the tests. But, due to local holidays I'll do it on monday.

[Marcelo Morais]

Created attachment 211105 [details]
patch

Skipping the tests for old inspector.

I also ran the tests for inspector-protocol and they have the same results for the old inspector and for the new inspector, some tests failed. I would like to create a specific bug for that and take a look at and try to fix the tests in a followup patch.

[Gyuyoung Kim]

Comment on attachment 211105 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=211105&action=review

> LayoutTests/platform/efl/TestExpectations:271
> +webkit.org/b/119559 inspector [ Skip ]

Please file a new bug for solving this issue.

[Marcelo Morais]

Created attachment 211202 [details]
patch

Created a new bug for update TestExpectations when new tests are available for the new inspector (https://bugs.webkit.org/show_bug.cgi?id=121096)

I also created another bug to check layout tests for inspector-protocol (https://bugs.webkit.org/show_bug.cgi?id=121094).

I expect look at these bugs soon.

[WebKit Commit Bot]

Comment on attachment 211202 [details]
patch

Clearing flags on attachment: 211202

Committed r155475: <http://trac.webkit.org/changeset/155475>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.