Bug 199369 – Web Inspector: Overlay: add page width/height display

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

Often one of the most useful parts of the page rulers is being able to see the width/height of the page.  There are ways of getting this information now (e.g. logging `window.innerWidth`), but they all involve some other user action.  It would be great to be able to see this information whenever rulers are shown so no extra actions are required of the user.

[Devin Rousso]

Created attachment 373585 [details]
Patch

[Joseph Pecoraro]

Got a screenshot?

[Joseph Pecoraro]

Comment on attachment 373585 [details]
Patch

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

> Source/WebCore/inspector/InspectorOverlay.cpp:844
> +        const UChar multiplicationSignUChar[] = { 0x00D7, 0 };

Is there any reason this needs to be a `UChar[]` and not just a `UChar`?

It seems like `makeString` would support a `UChar` since StringConcatenate has `template<> class StringTypeAdapter<UChar, void> {...}`. Unfortunately there aren't tests for it on `TestWebKitAPI/Tests/WTF/StringConcatenate.cpp` though. It does seem

[Devin Rousso]

Created attachment 373821 [details]
Patch

This does have one unfortunate side-effect right now, in that if the highlighted node is large enough, the node info will show _under_ the width/height in the top-left corner :(

[Devin Rousso]

Created attachment 373822 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 373821 [details]
Patch

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

r=me

> Source/WebCore/inspector/InspectorOverlay.cpp:77
> +static constexpr float viewportSizePadding = 5;

This constant seems like it could be put closer to the code as it won't be used elsewhere.

> Source/WebCore/inspector/InspectorOverlay.cpp:-79
> -    const UChar ellipsisUChar[] = { 0x2026, 0 };

Err, I was suggesting these be at the top level scope like so:

    const UChar ellipsis = 0x2026;
    const UChar multiplicationSign = 0x00D7;

Then I suspect it could just be used as expected:

    string.append(ellipsis);
    makeString(foo, multiplicationSign, bar);

I don't know why this is done weirdly in both `InspectorOverlay.cpp` and `InspectorDOMAgent.cpp`.

> Source/WebCore/inspector/InspectorOverlay.cpp:849
> +        FontCascadeDescription fontDescription;
> +        fontDescription.setOneFamily(m_page.settings().sansSerifFontFamily());
> +        fontDescription.setComputedSize(14);
> +
> +        FontCascade font(WTFMove(fontDescription), 0, 0);
> +        font.update(nullptr);

This seems to be a common pattern in the file. It might be nice to have a helper along the lines of:

    static FontCascade overlayFont(float computedSize)
    {
        FontCascadeDescription fontDescription;
        fontDescription.setFamilies({ "Menlo", m_page.settings().fixedFontFamily() });
        fontDescription.setComputedSize(11);

        FontCascade font(WTFMove(fontDescription), 0, 0);
        font.update(nullptr);
        return font;
    }

But maybe not until we have more uses.

[Devin Rousso]

Created attachment 375236 [details]
Patch

[Devin Rousso]

Created attachment 375237 [details]
[Image] After Patch is applied

[WebKit Commit Bot]

Comment on attachment 375236 [details]
Patch

Clearing flags on attachment: 375236

Committed r248053: <https://trac.webkit.org/changeset/248053>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/53778717>