New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Inspect Element with Firebug' not working with e10s enabled #8077

Open
janodvarko opened this Issue Feb 10, 2017 · 14 comments

Comments

Projects
None yet
4 participants
@janodvarko
Member

janodvarko commented Feb 10, 2017

This has been originally reported here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1335853

What did you do?

Hover over an object on a page, right-click-->Inspect Element with Firebug

What happened?

This toggles the inspector window on or off and when it toggles it on, it does not go to the html object - instead it's at the default body tag.

What should have happened?

It should not toggle the inspector window -- it should turn it on if it was off, but not toggle it off. It should also drill down to the object within the html.

Is there anything else we should know?

FireBug worked correctly. But you probably hear this a lot.


Looking at the Firebug code, the stack that leads to opening devtools is:

  • BrowserOverlay.js::startFirebug
  • BrowserOverlay.js::showMultiprocessNotification
  • BrowserOverlay.js::toggleDevTools
  • BrowserOverlay.js::showToolbox

And in the end we simply call DevTools.gDevTools.showToolbox(target);

Honza

@captainbrosset

This comment has been minimized.

Show comment
Hide comment
@captainbrosset

captainbrosset Feb 14, 2017

@janodvarko and I discussed about this. It is very confusing for people who are not using Firebug anymore to see 2 "inspect element" menu items in the list, and have one that doesn't work correctly.

We should keep the menu for people who have e10s off, because in that case they still use Firebug.
But for everyone else who don't have a choice, and are now using the built-in devtools, it seems just simpler to just hide the menu altogether.

I did some investigation, the menu item is added here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/firefox/browserMenu.js#L439-L447

And it is possible to check for multi-processes (e10s) with the BrowserOverlay.isMultiprocessEnabled function. A usage example is here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/firefox/browserMenu.js#L105

@janodvarko and I discussed about this. It is very confusing for people who are not using Firebug anymore to see 2 "inspect element" menu items in the list, and have one that doesn't work correctly.

We should keep the menu for people who have e10s off, because in that case they still use Firebug.
But for everyone else who don't have a choice, and are now using the built-in devtools, it seems just simpler to just hide the menu altogether.

I did some investigation, the menu item is added here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/firefox/browserMenu.js#L439-L447

And it is possible to check for multi-processes (e10s) with the BrowserOverlay.isMultiprocessEnabled function. A usage example is here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/firefox/browserMenu.js#L105

@SebastianZ

This comment has been minimized.

Show comment
Hide comment
@SebastianZ

SebastianZ Feb 14, 2017

Member

Created #8079 for this.

Sebastian

Member

SebastianZ commented Feb 14, 2017

Created #8079 for this.

Sebastian

@SebastianZ SebastianZ changed the title from Inspect Object not working to 'Inspect Element with Firebug' not working with e10s enabled Feb 14, 2017

@captainbrosset

This comment has been minimized.

Show comment
Hide comment
@captainbrosset

captainbrosset Feb 14, 2017

Oops, I didn't see @SebastianZ had jumped on this :)

Oops, I didn't see @SebastianZ had jumped on this :)

@SebastianZ

This comment has been minimized.

Show comment
Hide comment
@SebastianZ

SebastianZ Feb 14, 2017

Member

No problem. Thank you for working on this! As far as I can see, the only difference in our implementations is that I am additionally checking for Aurora, which actually may not be necessary.

@janodvarko Can you please have a look at the PRs?

Sebastian

Member

SebastianZ commented Feb 14, 2017

No problem. Thank you for working on this! As far as I can see, the only difference in our implementations is that I am additionally checking for Aurora, which actually may not be necessary.

@janodvarko Can you please have a look at the PRs?

Sebastian

@janodvarko

This comment has been minimized.

Show comment
Hide comment
@janodvarko

janodvarko Feb 14, 2017

Member

@janodvarko Can you please have a look at the PRs?

I already did, see my review comment.

Honza

Member

janodvarko commented Feb 14, 2017

@janodvarko Can you please have a look at the PRs?

I already did, see my review comment.

Honza

@captainbrosset

This comment has been minimized.

Show comment
Hide comment
@captainbrosset

captainbrosset Feb 16, 2017

Thanks @janodvarko and @SebastianZ ! What's the process now to get this landed and out to users?

Thanks @janodvarko and @SebastianZ ! What's the process now to get this landed and out to users?

@janodvarko

This comment has been minimized.

Show comment
Hide comment
@janodvarko

janodvarko Feb 16, 2017

Member

Thanks @janodvarko and @SebastianZ ! What's the process now to get this landed and out to users?

As soon as the patch is reviewed and landed, I'll build new xpi (version updated) and upload to AMO for review. The review can take couple of days and then it's available to all users.

Honza

Member

janodvarko commented Feb 16, 2017

Thanks @janodvarko and @SebastianZ ! What's the process now to get this landed and out to users?

As soon as the patch is reviewed and landed, I'll build new xpi (version updated) and upload to AMO for review. The review can take couple of days and then it's available to all users.

Honza

janodvarko added a commit that referenced this issue Feb 16, 2017

Merge pull request #8079 from firebug/issue-8077
Issue #8077: Hide 'Inspect Element with Firebug' menu item when e10s is enabled
@janodvarko

This comment has been minimized.

Show comment
Hide comment
@janodvarko

janodvarko Feb 16, 2017

Member

New version (2.0.19) is now waiting for review on AMO.

Honza

Member

janodvarko commented Feb 16, 2017

New version (2.0.19) is now waiting for review on AMO.

Honza

@SebastianZ

This comment has been minimized.

Show comment
Hide comment
@SebastianZ

SebastianZ Feb 17, 2017

Member

@janodvarko thank you for pushing the new version online! Can you please also

  1. add a note to the AMO site telling people that Firebug is discontinued and people get automatically switched to the DevTools and link to the migration guide?
  2. update https://getfirebug.com/downloads/ to the new version?

@captainbrosset I don't want to discourage you, but the comments on AMO about "the new Firebug version" are pretty bad. The DevTools urgently need some UX improvements. I'll try to help if I can find some spare time.

Sebastian

Member

SebastianZ commented Feb 17, 2017

@janodvarko thank you for pushing the new version online! Can you please also

  1. add a note to the AMO site telling people that Firebug is discontinued and people get automatically switched to the DevTools and link to the migration guide?
  2. update https://getfirebug.com/downloads/ to the new version?

@captainbrosset I don't want to discourage you, but the comments on AMO about "the new Firebug version" are pretty bad. The DevTools urgently need some UX improvements. I'll try to help if I can find some spare time.

Sebastian

@janodvarko

This comment has been minimized.

Show comment
Hide comment
@janodvarko

janodvarko Feb 20, 2017

Member

add #1) AMO updated.
add #2) I am doing this as soon as the new version passes AMO review.

Honza

Member

janodvarko commented Feb 20, 2017

add #1) AMO updated.
add #2) I am doing this as soon as the new version passes AMO review.

Honza

@janodvarko

This comment has been minimized.

Show comment
Hide comment
@janodvarko

janodvarko Feb 20, 2017

Member

the comments on AMO about "the new Firebug version" are pretty bad. The DevTools
urgently need some UX improvements.

Agree on this.

I am currently focusing on the Netmonitor panel (as part of Netmonitor.html project, i.e. removing XUL and Chrome API). My plan for next steps is triaging all firebug-gaps reports and collect UI/UX suggestions. As soon as the Net panel refactoring phase is done we can start improve the UI.

Later, we might want to do similar things for other panels.

Honza

Member

janodvarko commented Feb 20, 2017

the comments on AMO about "the new Firebug version" are pretty bad. The DevTools
urgently need some UX improvements.

Agree on this.

I am currently focusing on the Netmonitor panel (as part of Netmonitor.html project, i.e. removing XUL and Chrome API). My plan for next steps is triaging all firebug-gaps reports and collect UI/UX suggestions. As soon as the Net panel refactoring phase is done we can start improve the UI.

Later, we might want to do similar things for other panels.

Honza

@SebastianZ

This comment has been minimized.

Show comment
Hide comment
@SebastianZ

SebastianZ Feb 20, 2017

Member

My plan for next steps is triaging all firebug-gaps reports and collect UI/UX suggestions. As soon as the Net panel refactoring phase is done we can start improve the UI.

For reference, I listed the (in my eyes) most pressing Firebug gaps in bug 991806 comment 20. As there were rather few comments until then, it's rather a guess, though, and may not be up-to-date. E.g. several people were arguing that the DevTools are missing FirePath features.
Also, this list doesn't contain UI/UX issues unrelated to Firebug, e.g. bug 1324254, which was mentioned several times (and is somewhat related to this issue here).

Sebastian

Member

SebastianZ commented Feb 20, 2017

My plan for next steps is triaging all firebug-gaps reports and collect UI/UX suggestions. As soon as the Net panel refactoring phase is done we can start improve the UI.

For reference, I listed the (in my eyes) most pressing Firebug gaps in bug 991806 comment 20. As there were rather few comments until then, it's rather a guess, though, and may not be up-to-date. E.g. several people were arguing that the DevTools are missing FirePath features.
Also, this list doesn't contain UI/UX issues unrelated to Firebug, e.g. bug 1324254, which was mentioned several times (and is somewhat related to this issue here).

Sebastian

@inspector71

This comment has been minimized.

Show comment
Hide comment
@inspector71

inspector71 Mar 3, 2017

I resent being pushed an update like this. It's arguably one step short of arbitrarily, remotely disabling functionality in people's software considering Firefox has never given users the UI to downgrade an addon-version, but only the ability to disable or remove.

FWIW I've been trying to switch over to devtools since it's being forced on to all developers who want to remain loyal to Firefox. It's just inferior to Firebug - STILL - after years of development (and the corresponding lack of development in Firebug), The Firebug theme for Devtools isn't very accurate at all.

Is it realistic for a front-end web dev with minimal Firefox ('chrome'-side) development experience to contribute to DevTools through fixing the inconsistencies in the Firebug DevTools theme? I guess I"ll see if I can try starting with Stylish.

inspector71 commented Mar 3, 2017

I resent being pushed an update like this. It's arguably one step short of arbitrarily, remotely disabling functionality in people's software considering Firefox has never given users the UI to downgrade an addon-version, but only the ability to disable or remove.

FWIW I've been trying to switch over to devtools since it's being forced on to all developers who want to remain loyal to Firefox. It's just inferior to Firebug - STILL - after years of development (and the corresponding lack of development in Firebug), The Firebug theme for Devtools isn't very accurate at all.

Is it realistic for a front-end web dev with minimal Firefox ('chrome'-side) development experience to contribute to DevTools through fixing the inconsistencies in the Firebug DevTools theme? I guess I"ll see if I can try starting with Stylish.

@SebastianZ

This comment has been minimized.

Show comment
Hide comment
@SebastianZ

SebastianZ Mar 6, 2017

Member

Is it realistic for a front-end web dev with minimal Firefox ('chrome'-side) development experience to contribute to DevTools through fixing the inconsistencies in the Firebug DevTools theme?

Yes, most parts of the DevTools UI are pure HTML, CSS and JavaScript. You should read how to get involved at https://wiki.mozilla.org/DevTools/GetInvolved, though.

Sebastian

Member

SebastianZ commented Mar 6, 2017

Is it realistic for a front-end web dev with minimal Firefox ('chrome'-side) development experience to contribute to DevTools through fixing the inconsistencies in the Firebug DevTools theme?

Yes, most parts of the DevTools UI are pure HTML, CSS and JavaScript. You should read how to get involved at https://wiki.mozilla.org/DevTools/GetInvolved, though.

Sebastian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment