Skip to content
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

Add Web Inspector docs (WIP) #58

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jdatapple
Copy link
Member

Ported Web Inspector docs from trac.webkit.org

  • docs/Deep Dive/Web Inspector/
  • docs/Deep Dive/Web Inspector/AnIntroduction.md
  • docs/Deep Dive/Web Inspector/Contributing.md
  • docs/Deep Dive/Web Inspector/RemoteInspectorWebKitGTKandWPE.md
  • docs/Deep Dive/Web Inspector/WebInspectorCodeStyle.md
  • docs/Deep Dive/Web Inspector/WebInspectorDebugging.md
  • docs/Deep Dive/Web Inspector/WebInspectorTests.md
  • docs/Deep Dive/Web Inspector/WebReplayMechanics.md

Copy link
Member

@stwrt stwrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stylistic comments, but I don't have anything that is blocking.

Really appreciate you taking the time to port this over.


### Remote Web Inspector on GTK+ and EFL ports

* For the GTK and EFL ports, this can be enabled with an environment variable. Check the documentation on EnvironmentVariables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor quip extra space

Computer2 # MiniBrowser http://${ip.ad.dre.ss}:${port}
```

* The very same version of WebKit has to be used on the other side
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

By inspecting the CSS property names in the second-level inspector, we quickly find that the colors are defined by rules in `Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css`.
To create and submit a patch with our changes, we must to create an accompanying Bugzilla bug, and compute the diff of our changes against WebKit trunk.

## Create / Update a Bug Report
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already have the content on how to build WebKit elsewhere, but this is not a big deal.


4. Edit `Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css` within the repo.

5. Run `make -C Source/WebInspectorUI release` to copy files from `Source/WebInspectorUI/UserInterface` to the build directory. Do it after every time you modify Inspector's files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super helpful though. We should keep this.

@@ -0,0 +1,252 @@
# Web Replay Mechanics

*Brian Burg <burg@cs.washington.edu>*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a little strange just having the name at the top here.


## Create / Update a Bug Report

* [Existing Web Inspector Bugs](https://bugs.webkit.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&component=Web+Inspector&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0=)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space


Various HTML and JavaScript properties, including length of text nodes, offsetWidth/Height, class names, and parent/sibling information are vieweable in the Properties pane.

See [Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html) for more details on other panels of the Web Inspector.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link is out of date and leads to a 404. The most up to date WI documentation is at https://webkit.org/web-inspector/ afaik.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a overview of Web Inspector at:
https://developer.apple.com/documentation/safari-developer-tools/web-inspector
It's helpful to mention because its sibling pages detail how to inspect various types of targets (Simulators, remote devices, etc)

But I agree, we should point to the Web Inspector Reference at:
https://webkit.org/web-inspector/


### Apple Web Inspector Remote Experiment

[Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link does not work

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove the link altogether and keep the other two paragraphs for historical reference.

-[Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html)


### z-index

Z-index variables are defined in [Variables.css](https://trac.webkit.org/browser/trunk/Source/WebInspectorUI/UserInterface/Views/Variables.css). Usage example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should update this to GitHub link

For the Mac port, set the following defaults to allow inspecting the inspector.

```
defaults write NSGlobalDomain WebKitDebugDeveloperExtrasEnabled -bool YES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK if this still works but this can now be done from the web inspector UI.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Developers can now enable "Allow Inspecting Web Inspector" in the Web Inspector Settings > Experimental tab.
Screenshot 2023-09-29 at 16 08 11

I don't know if the defaults pref is relevant for other ports. I suspect not.


Under the Style pane we show all the CSS rules that apply to the focused node. These rules are listed in cascade order with overridden properties striked-out—letting you truly see how cascading stylesheets affect the page layout. All shorthand properties have a disclosure-triangle to show and hide the expanded properties created by the shorthand.

The Metrics pane provides a quick visual look at how margins, borders and padding affect the current node.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics are now under the Computed pane. Personally, I would just remove all this info and link to the documentation as a single source of truth to avoid it getting outdated again, but I’m a new contributor so those are just my thoughts.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to agree. A paragraph or two about the general scope of Web Inspector is sufficient, then a add a link to the canonical Web Inspector Reference: https://webkit.org/web-inspector/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a question mark in the help links, as it may not be obvious for people unfamiliar with Web Inspector.

"help links with a question mark"

@@ -0,0 +1,281 @@
# Web Inspector Syle Guide
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo


| | Mac | Windows / Linux |
|-----------------------------|-----|-----------------|
| Next Panel | ⌘] | Ctrl-] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ⌘} and ``⌘{`.

This information is quite similar to here.
https://webkit.org/web-inspector/keyboard-shortcuts/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web Replay was removed from WebKit at some point. There's the meta bug for adding it, but I can't find the definitive bug which removed it. Perhaps @burg can point to it.

This documentation file should be archived.


### Safari

* Enable the Develop menu option in the Advanced preferences.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Enable the Develop menu option in the Advanced preferences.
+ Enable ["Show features for web developers"](https://developer.apple.com/documentation/safari-developer-tools/enabling-developer-features) in the Advanced tab of Safari settings.


### Apple Web Inspector Remote Experiment

[Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove the link altogether and keep the other two paragraphs for historical reference.

-[Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html)


Various HTML and JavaScript properties, including length of text nodes, offsetWidth/Height, class names, and parent/sibling information are vieweable in the Properties pane.

See [Safari User Guide for Web Developers](http://developer.apple.com/safari/library/documentation/AppleApplications/Conceptual/Safari_Developer_Guide/UsingtheWebInspector/UsingtheWebInspector.html) for more details on other panels of the Web Inspector.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a overview of Web Inspector at:
https://developer.apple.com/documentation/safari-developer-tools/web-inspector
It's helpful to mention because its sibling pages detail how to inspect various types of targets (Simulators, remote devices, etc)

But I agree, we should point to the Web Inspector Reference at:
https://webkit.org/web-inspector/


Under the Style pane we show all the CSS rules that apply to the focused node. These rules are listed in cascade order with overridden properties striked-out—letting you truly see how cascading stylesheets affect the page layout. All shorthand properties have a disclosure-triangle to show and hide the expanded properties created by the shorthand.

The Metrics pane provides a quick visual look at how margins, borders and padding affect the current node.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to agree. A paragraph or two about the general scope of Web Inspector is sufficient, then a add a link to the canonical Web Inspector Reference: https://webkit.org/web-inspector/

For the Mac port, set the following defaults to allow inspecting the inspector.

```
defaults write NSGlobalDomain WebKitDebugDeveloperExtrasEnabled -bool YES

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Developers can now enable "Allow Inspecting Web Inspector" in the Web Inspector Settings > Experimental tab.
Screenshot 2023-09-29 at 16 08 11

I don't know if the defaults pref is relevant for other ports. I suspect not.


## How to Debug Tests

In general, the strategies for [wiki:"WebInspectorDebugging" debugging the Web Inspector] and debugging WebCore/WebKit2 apply the same to debugging inspector tests. Sometimes, tests can be more difficult to debug because the test harness' marshalling code can be broken by incorrectly written tests or bugs in the test harness. The test stubs provide several flags that enable extra or more reliable logging for debug purposes. Flags can be set in the corresponding `Test/TestStub.html` file for all test runs, or at the top of a `test()` method to only affect one test.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[wiki:"WebInspectorDebugging" debugging the Web Inspector]
Needs updated link referencing the WebInspectorDebugging.md file from this patch.

@stwrt
Copy link
Member

stwrt commented Feb 7, 2024

Not sure why it is suddenly showing 20 commits. Maybe you need to rebase?

@rcaliman-apple
Copy link

Not sure why it is suddenly showing 20 commits. Maybe you need to rebase?

I did. But I guess I messed up elsewhere with my git commands.

I'll likely send it another PR. Sorry about the noise! 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants