Commit 6bed5799 authored by Julian Descottes's avatar Julian Descottes
Browse files

Bug 1783893 - [devtools] Changes view should rely on the computed property name r=nchevobbe 

Similar to the fix done for values in Bug 1590031, now focused on property names.

Differential Revision: https://phabricator.services.mozilla.com/D154124
parent 151c7096

devtools/client/inspector/changes/test/browser.ini

+1 −0
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ support-files = 
[browser_changes_copy_all_changes.js]

[browser_changes_copy_declaration.js]

[browser_changes_copy_rule.js]

[browser_changes_declaration_add_special_character.js]

[browser_changes_declaration_disable.js]

[browser_changes_declaration_duplicate.js]

[browser_changes_declaration_edit_value.js]

devtools/client/inspector/changes/test/browser_changes_declaration_add_special_character.js

0 → 100644
+76 −0
Original line number Diff line number Diff line 
/* Any copyright is dedicated to the Public Domain.

 http://creativecommons.org/publicdomain/zero/1.0/ */



"use strict";



// Test that adding new CSS properties with special characters in the property

// name does note create duplicate entries.



const PROPERTY_NAME = '"abc"';

const INITIAL_VALUE = "foo";

// For assertions the quotes in the property will be escaped.

const EXPECTED_PROPERTY_NAME = '\\"abc\\"';



const TEST_URI = `

  <style type='text/css'>

    div {

      color: red;

    }

  </style>

  <div>test</div>

`;



add_task(async function addWithSpecialCharacter() {

  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));

  const { inspector, view: ruleView } = await openRuleView();

  const { document: doc, store } = selectChangesView(inspector);



  await selectNode("div", inspector);



  const ruleEditor = getRuleViewRuleEditor(ruleView, 1);

  const editor = await focusEditableField(ruleView, ruleEditor.closeBrace);



  const input = editor.input;

  input.value = `${PROPERTY_NAME}: ${INITIAL_VALUE};`;



  let onTrackChange = waitForDispatch(store, "TRACK_CHANGE");

  info("Pressing return to commit and focus the new value field");

  const onModifications = ruleView.once("ruleview-changed");

  EventUtils.synthesizeKey("VK_RETURN", {}, ruleView.styleWindow);

  await onModifications;

  await onTrackChange;

  await assertAddedDeclaration(doc, EXPECTED_PROPERTY_NAME, INITIAL_VALUE);



  let newValue = "def";

  info(`Change the CSS declaration value to ${newValue}`);

  const prop = getTextProperty(ruleView, 1, { [PROPERTY_NAME]: INITIAL_VALUE });

  onTrackChange = waitForDispatch(store, "TRACK_CHANGE");

  // flushCount needs to be set to 2 once when quotes are involved.

  await setProperty(ruleView, prop, newValue, { flushCount: 2 });

  await onTrackChange;

  await assertAddedDeclaration(doc, EXPECTED_PROPERTY_NAME, newValue);



  newValue = "123";

  info(`Change the CSS declaration value to ${newValue}`);

  onTrackChange = waitForDispatch(store, "TRACK_CHANGE");

  await setProperty(ruleView, prop, newValue);

  await onTrackChange;

  await assertAddedDeclaration(doc, EXPECTED_PROPERTY_NAME, newValue);

});



/**

 * Check that we only received a single added declaration with the expected

 * value.

 */

async function assertAddedDeclaration(doc, expectedName, expectedValue) {

  await waitFor(() => {

    const addDecl = getAddedDeclarations(doc);

    return (

      addDecl.length == 1 &&

      addDecl[0].value == expectedValue &&

      addDecl[0].property == expectedName

    );

  }, "Got the expected declaration");

  is(getAddedDeclarations(doc).length, 1, "Only one added declaration");

  is(getRemovedDeclarations(doc).length, 0, "No removed declaration");

}

devtools/server/actors/style-rule.js

+6 −3
Original line number Diff line number Diff line
@@ -946,7 +946,8 @@ const StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { 
      commentOffsets,

    } = oldDeclarations[index] || {};



    const { value: currentValue } = newDeclarations[index] || {};

    const { value: currentValue, name: currentName } =

      newDeclarations[index] || {};

    // A declaration is disabled if it has a `commentOffsets` array.

    // Here we type coerce the value to a boolean with double-bang (!!)

    const prevDisabled = !!commentOffsets;
@@ -961,8 +962,10 @@ const StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { 
        data.type = prevValue ? "declaration-add" : "declaration-update";

        // If `change.newName` is defined, use it because the property is being renamed.

        // Otherwise, a new declaration is being created or the value of an existing

        // declaration is being updated. In that case, use the provided `change.name`.

        const name = change.newName ? change.newName : change.name;

        // declaration is being updated. In that case, use the currentName computed

        // by the engine.

        const changeName = currentName || change.name;

        const name = change.newName ? change.newName : changeName;

        // Append the "!important" string if defined in the incoming priority flag.



        const changeValue = currentValue || change.value;

devtools/shared/commands/resource/tests/browser_resources_css_changes.js

+29 −8
Original line number Diff line number Diff line
@@ -46,20 +46,34 @@ add_task(async function() { 
  );



  info(

    "Check whether ResourceCommand catches CSS change after the property changed"

    "Check whether ResourceCommand catches CSS changes after the property was renamed and updated"

  );

  await setProperty(style.rule, 0, "background-color", "pink");



  // RuleRewriter:apply will not support a simultaneous rename + setProperty.

  // Doing so would send inconsistent arguments to StyleRuleActor:setRuleText,

  // the CSS text for the rule will not match the list of modifications, which

  // would desynchronize the Changes view. Thankfully this scenario should not

  // happen when using the UI to update the rules.

  await renameProperty(style.rule, 0, "color", "background-color");

  await waitUntil(() => availableResources.length === 2);

  assertResource(

    availableResources[1],

    { index: 0, property: "background-color", value: "pink" },

    { index: 0, property: "background-color", value: "black" },

    { index: 0, property: "color", value: "black" }

  );



  await setProperty(style.rule, 0, "background-color", "pink");

  await waitUntil(() => availableResources.length === 3);

  assertResource(

    availableResources[2],

    { index: 0, property: "background-color", value: "pink" },

    { index: 0, property: "background-color", value: "black" }

  );



  info("Check whether ResourceCommand catches CSS change of disabling");

  await setPropertyEnabled(style.rule, 0, "background-color", false);

  await waitUntil(() => availableResources.length === 3);

  assertResource(availableResources[2], null, {

  await waitUntil(() => availableResources.length === 4);

  assertResource(availableResources[3], null, {

    index: 0,

    property: "background-color",

    value: "pink",
@@ -67,9 +81,9 @@ add_task(async function() { 


  info("Check whether ResourceCommand catches CSS change of new property");

  await createProperty(style.rule, 1, "font-size", "100px");

  await waitUntil(() => availableResources.length === 4);

  await waitUntil(() => availableResources.length === 5);

  assertResource(

    availableResources[3],

    availableResources[4],

    { index: 1, property: "font-size", value: "100px" },

    null

  );
@@ -79,11 +93,12 @@ add_task(async function() { 
  await resourceCommand.watchResources([resourceCommand.TYPES.CSS_CHANGE], {

    onAvailable: resources => existingResources.push(...resources),

  });

  await waitUntil(() => existingResources.length === 4);

  await waitUntil(() => existingResources.length === 5);

  is(availableResources[0], existingResources[0], "1st resource is correct");

  is(availableResources[1], existingResources[1], "2nd resource is correct");

  is(availableResources[2], existingResources[2], "3rd resource is correct");

  is(availableResources[3], existingResources[3], "4th resource is correct");

  is(availableResources[4], existingResources[4], "4th resource is correct");



  targetCommand.destroy();

  await client.close();
@@ -117,6 +132,12 @@ async function setProperty(rule, index, property, value) { 
  await modifications.apply();

}



async function renameProperty(rule, index, oldName, newName, value) {

  const modifications = rule.startModifyingProperties({ isKnown: true });

  modifications.renameProperty(index, oldName, newName);

  await modifications.apply();

}



async function createProperty(rule, index, property, value) {

  const modifications = rule.startModifyingProperties({ isKnown: true });

  modifications.createProperty(index, property, value, "", true);