Closed Bug 1904752 Opened 1 year ago Closed 1 year ago

Enter single quote in property value isn't handled properly by the Rule rewriter

Categories

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

Product:
DevTools
Component:
Inspector: Rules
Version:
Firefox 127
Type:
defect

Tracking

(firefox-esr115 unaffected, firefox127 wontfix, firefox128 fixed, firefox129 fixed)

Status:
RESOLVED FIXED
Milestone:
129 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox127 --- wontfix
firefox128 --- fixed
firefox129 --- fixed

People

(Reporter: 6k64x4ma, Assigned: nchevobbe)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Bug 1904752 - [devtools] Fix handling of single-char quote in InspectorCSSParserWrapper#performEOFFixup. r=#devtools-reviewers.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1904752 - [devtools] Fix handling of single-char quote in InspectorCSSParserWrapper#performEOFFixup. .
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:128.0) Gecko/20100101 Firefox/128.0

Steps to reproduce:

  1. Go to GitHub page e.g. https://github.com/mozilla/gecko-dev
  2. Open the Inspector and select <body>.
  3. Click on font-family value.
  4. Replace the value with ".

Actual results:

  • The styles of the page is broken.
    Pressing ESC key to cancel the edit does not return the page to its correct state.

Expected results:

  • The page should not be broken like this.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=49aba74b154f99c1b31e609b596a468232697e5a&tochange=3bc4640f938f9922129f742e7f4556f203b0461d

Flags: needinfo?(nchevobbe)

Thanks for the report.
I can reproduce on a simpler case: data:text/html,<meta charset=utf8><style>h1 { font-family: "Arial"; color: red; }</style><h1>Hello
When validating the " value, the stylesheet text is set to

h1 {
	font-family: ";}
Flags: needinfo?(nchevobbe)
Keywords: regression
Regressed by: 1890493
Regressed by: 1889363
No longer regressed by: 1890493
Summary: Editing font-family breaks GitHub styles → Enter single quote in property value isn't handled properly by the Rule rewriter
Severity: -- → S3
Status: UNCONFIRMED → NEW
status-firefox127: --- → wontfix
status-firefox128: --- → affected
status-firefox129: --- → affected
Ever confirmed: true
Priority: -- → P2

Set release status flags based on info from the regressing bug 1889363

status-firefox-esr115: --- → unaffected

When tracking tokens, we weren't handling the case of unterminated,
single-char quote.
Test cases are added to cover the fix.

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/95b4bb59f7c3 [devtools] Fix handling of single-char quote in InspectorCSSParserWrapper#performEOFFixup. r=devtools-reviewers,jdescottes.

https://hg.mozilla.org/mozilla-central/rev/95b4bb59f7c3

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox129: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch

When tracking tokens, we weren't handling the case of unterminated,
single-char quote.
Test cases are added to cover the fix.

Original Revision: https://phabricator.services.mozilla.com/D214917

Attachment #9409888 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: setting property value with quotes in the DevTools rules view will mess up the rest of the stylesheet, which could change the whole page styling. This can't be reverted, unless the user reloads the page, which is extremely annoying
  • Code covered by automated testing: yes
  • Fix verified in Nightly: no
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: -
  • Risk associated with taking this patch: low
  • Explanation of risk level: one liner fix, covered by automated test, devtools only change
  • String changes made/needed: -
  • Is Android affected?: no
Attachment #9409888 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: