Closed Bug 1521085 Opened 10 months ago Closed 5 months ago

Rules inspector - pasting does not add invalid and past invalid properties

Categories

(DevTools :: Inspector: Rules, defect, P2)

defect

Tracking

(firefox-esr60 wontfix, firefox-esr68 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 wontfix, firefox68 wontfix, firefox69 verified, firefox70 verified)

VERIFIED FIXED
Firefox 70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: cfogel, Assigned: daisuke)

Details

(Keywords: regression, Whiteboard: [dt-q])

Attachments

(3 files)

Attached image devT_invalidPaste.gif

Affected versions

  • 65.0b12
  • 66.0a1 (2019-01-17)
  • 60.0.1ESR
  • 64.0.2

Affected platforms

  • Windows 10;
  • macOS 10.12.6;
  • Ubuntu 18.04;

Steps to reproduce

  1. Launch Firefox, open the devTools inpsector;
  2. For any element paste in the rules section the following:
    font-size:30px; 1:1; color:red;

Expected result

  • properties and values are pasted in the rules section for the element;
  • invalid properties are marked as invalid;

Actual result

  • past the invalid property nothing is pasted;

Regression range

Additional notes

  • pasting the string from step 2 in the values section keeps the value box border displayed (as if editting) even when jumping to the next property;
  • attached recording with the issue;
Component: Inspector → CSS Rules Inspector
Whiteboard: [dt-q]
Has Regression Range: --- → yes
Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Attachment #9076483 - Attachment description: Bug 1521085: Catch an error occurring when the property name can be an index of array. r?gl → Bug 1521085: Return false if the property name is an invalid identifier. r?gl
Attachment #9076484 - Attachment description: Bug 1521085: Add a test which add an invalid numeric property. r?gl → Bug 1521085: Add a test which add an invalid identifier. r?gl
Attachment #9076484 - Attachment description: Bug 1521085: Add a test which add an invalid identifier. r?gl → Bug 1521085: Add a test which add a property which is an invalid identifier. r?gl
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a82845957b4
Return false if the property name is an invalid identifier. r=gl
https://hg.mozilla.org/integration/autoland/rev/624428794690
Add a test which add a property which is an invalid identifier. r=gl
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Verified with 70.0a1 (2019-07-11) on Windows 10, macOS 10.11, Ubuntu 18.04.

Status: RESOLVED → VERIFIED

Is this something we should consider uplifting to Beta for DevEdition 69?

Flags: needinfo?(daisuke)
Flags: in-testsuite+

Hi Ryan,
I think, since this is an actual bug and does not affect any other components, we better to uplift if possible.

Flags: needinfo?(daisuke)

Please nominate the patch for Beta approval in that case :)

Flags: needinfo?(daisuke)

Comment on attachment 9076483 [details]
Bug 1521085: Return false if the property name is an invalid identifier. r?gl

Beta/Release Uplift Approval Request

  • User impact if declined: If user inputs non identifier text in the property name field, user will not be able to edit the property correctly.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change just adds one logic which checks the validity of property name. Also, this does not affect except DevTools inspector.
  • String changes made/needed:
Flags: needinfo?(daisuke)
Attachment #9076483 - Flags: approval-mozilla-beta?
Attachment #9076484 - Flags: approval-mozilla-beta?

Comment on attachment 9076483 [details]
Bug 1521085: Return false if the property name is an invalid identifier. r?gl

Adds a check to avoid the Devtools Inspector getting into a bad state. Approved for 69.0b7.

Attachment #9076483 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9076484 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified with 69.0b7 as well.

You need to log in before you can comment on or make changes to this bug.