1538728 - Changed rule that contains webkit element not copied entirely via the Copy Rule hover button

Status: RESOLVED INVALID

(DevTools :: Inspector: Changes, defect)

Description

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Affected versions

• 67.0b4, 68.0a1 (2019-03-25)

Affected platforms

• Windows 10; macOS 10.12, Ubuntu 16.04x64

Steps to reproduce

1. Launch Firefox and open the Changes tab from the devTools inspector;
2. Go to https://ultimatemotorcycling.com/2019/03/23/arlen-ness-passes-1939-2019/
3. In the rules section, find webkit-text-size-adjust:>100 and delete the property completly
4. Repeat step 3 for font-family: sans-serif;
5. Go to the Changes tab and hover over the rule and click the Copy Rule button.

Expected result

• The removed rule and its components are copied in its entirety.

Actual result

• The whole rule and especially the webkit content is not copied to clipboard when pasting it in a text document or https://pastebin.com/.

Regression range

• not a regression;

Additional notes

• Value that is actually pasted is :

html { -ms-text-size-adjust:100%; }

Comment 1

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

:rcaliman

After further investigating this, I have concluded that the actual issue is that the Copy Rule hover button does not copy the actual changes in the third pane, but instead it copies the declaration values from the second pane.

STR:

1.Go to http://www.freeclipboardviewer.com/.
2.Open Changes CTRL+Shift+C
3.Change and delete values so as to populate the Changes pane.
4.Hover the changes to get the Copy Rule button then click it.
5.Paste the values in a document or Pastebin.
6.Notice that the values pasted are the declarations form the second pane instead of the changes in the third.

Here is a screencast of the issue:
https://drive.google.com/file/d/1iP85h193ZQzgD6ASpCwycXclhve5F259/view

Comment 2

[Razvan Caliman [:rcaliman]]

I have concluded that the actual issue is that the Copy Rule hover button does not copy the actual changes in the third pane, but instead it copies the declaration values from the second pane.

This is the intended behavior of the Copy Rule button: to copy the entire contents of the target CSS Rule, including unchanged declarations, so that a user may overwrite their original code in their editor. The Copy All Changes button will copy only the changed declarations as shown in the Changes panel (which is what you probably expected from Copy Rule).

The original issue with the unexpected -ms-text-size-adjust:100%; in the clipboard still stands. Most likely, the issue is that the -ms- prefixed property does not render in the Rules panel, but it exists in the CSS text.

The Copy Rule in this scenario does the right thing: it copies the end result of the changed CSS rule which is the rule without the deleted declarations but with the stray -ms-text-size-adjust:100%; that didn't originally show up in the Rules view.

Comment 3

[Razvan Caliman [:rcaliman]]

Narrowed down the issue to -ms--prefixed properties not showing up for inherited properties in the Rules panel. But I'm not sure that the behavior is wrong, though. Details below.

First, the updated STR:

1. paste this in the address bar:

data:text/html,<style>html {font-size:1em;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;}</style>

2. open DevTools and inspect the body element
3. notice the inherited properties from the html element (the -ms-text-size-adjust:100%; declaration is missing).
4. delete all declarations for the inherited html {} rule
5. switch to the Changes panel and click on Copy Rule for the changed html {}
6. paste the clipboard into a text file.

The original issue isn't a bug, as explained in Comment 2. The Copy Rule button copies the result of the changed html {} rule whereby the -ms-text-size-adjust:100%; is the only thing remaining because it didn't show up in the Rules view to be deleted.

The reason why I think not showing the -ms-text-size-adjust:100% declaration as inherited is correct is that it's not supported by Firefox so it can't be inherited. Likewise, -webkit-text-size-adjust is not supported either. Showing it is wrong as well. The only inherited declaration in this scenario is font-size: 1em.

Pinging Martin Balfanz (PM) to double-check my interpretation and confirm if we should log a separate bug for the misleading inherited prefixed properties. Also to draw attention to the example of a user who confuses what Copy Rule button does and whether we need to adjust the UX.

Comment 4

[Martin Balfanz [:mbalfanz]]

I think we should log a separate bug to address the issue in the rules view. However, in my opinion unsupported properties should show up as inherited. The -webkit-text-size-adjust behavior is correct, and -ms-text-size-adjust should show up as well.

Knowing that it might not be technically correct, I think this is the best solution for the user. We would show the whole rule and cross out/grey out properties that don't apply. That would also be in line with Safari and Chrome.

As a side effect, it would help to better explain the results of "copy rule".

Comment 5

[Patrick Brosset <:pbro>]

Here is the relevant piece of code where we filter these properties out: https://searchfox.org/mozilla-central/rev/a7315d78417179b151fef6108f2bce14786ba64d/devtools/client/inspector/rules/models/rule.js#557-570
So, we do this on purpose. As Martin pointed out, Chrome and Safari do not do this.

Comment 6

[Razvan Caliman [:rcaliman]]

In light of this discussion, I conclude that this issue is not a bug with the Changes panel so I will close it.

I opened a separate Bug 1539899 for the Rules panel to treat the underlying cause for the confusion (missing declarations from the inherited rule) and to align Firefox implementation with Chrome and Safari.