Bug 169896 – Web Inspector: Editing local storage keys/values starts with truncated strings

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

[Nikita Vasilyev]

rdar://problem/14927212

Summary:
When you anticipate a website that needs to use LocalStorage or SessionStorage above a certain amount of characters, the browser saves the content normally in the browser. By accessing the Developer mode and trying to edit the content, what happens is the last part of the saved data is replaced with a "…" character and all other info is lost.
You can no longer edit this data, since most of it is lost and the (web) application is probably broken now.

Steps to Reproduce:
1. Open Safari
2. Visit website that uses local storage and saves a large (over 50 characters or so value) [Better keep Safari in a small viewport]
3. Right click anywhere
4. Click "Inspect Element"
5. Click the "Resources" tab
6. "Click LocalStorage or SessionStorage"
7. Locate a key with a value ending in "…" (chopped to fit screen)
8. Double click the value to edit
9. Click anywhere to save it.

Expected Results:
Safari was supposed to present the full data when the user double clicks to edit the key.

Actual Results:
Safari presents only the beginning of the data, with a "…" instead of the tailing characters. LocalStorage is edited and the web app is probably broken now.

Notes:
When double clicking, restore value to the full key, and not simply allow editing with the "…" in place.

[Nikita Vasilyev]

Created attachment 304963 [details]
Patch

[Nikita Vasilyev]

Created attachment 304965 [details]
[Animated GIF] With patch applied

[Matt Baker]

Nice, I noticed this a while back.

[Matt Baker]

Comment on attachment 304963 [details]
Patch

r-, for some reason `text-overflow: ellipsis` isn't restored after cancel/commit.

[Nikita Vasilyev]

(In reply to comment #4)
> Comment on attachment 304963 [details]
> Patch
> 
> r-, for some reason `text-overflow: ellipsis` isn't restored after
> cancel/commit.

After canceling

<td class="value-column">
    <div class="cell-content">foo</div>
</td>

gets replaced with

<td class="value-column">foo</td>

🤦‍♂️

[Nikita Vasilyev]

Created attachment 305062 [details]
Patch

I've tried making <div class=cell-content> editable instead of <td>, but it appeared to be difficult to style to make it look the way <td class=editing> looks now.

I ended up re-creating <div class=cell-content> after every edit.

[Nikita Vasilyev]

Created attachment 305138 [details]
Patch

As per an offline discussion with Brian, there should be only one code path for creating <div class=cell-content>.

[Build Bot]

Comment on attachment 305138 [details]
Patch

Attachment 305138 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3390911

New failing tests:
media/video-main-content-autoplay.html

[Build Bot]

Created attachment 305149 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Nikita Vasilyev]

Created attachment 305203 [details]
Patch

Unrelated flaky test. Re-uploading the patch.

[Blaze Burg]

Comment on attachment 305203 [details]
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:13
> +        since it may get removed while editing.

You should mention that div.cell-content is removed because the <td> itself is contenteditable (using the fancy -webkit-user-modify:read-write-plaintext-only), not the inner div.cell-content.

[Nikita Vasilyev]

Created attachment 305204 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 305204 [details]
Patch

Clearing flags on attachment: 305204

Committed r214308: <http://trac.webkit.org/changeset/214308>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.