Closed Bug 1497950 Opened 11 months ago Closed 11 months ago

Font editor doesn't work in 2-pane mode; missing selectedRule

Categories

(DevTools :: Inspector: Fonts, defect, P1)

defect

Tracking

(firefox-esr60 unaffected, firefox62 unaffected, firefox63 unaffected, firefox64 verified)

VERIFIED FIXED
Firefox 64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- verified

People

(Reporter: rcaliman, Assigned: rcaliman)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

The current iteration of the Fonts panel requires an instance of the Rules view
in order to get access to the element's rules. 

In 2-pane mode, when the Fonts panel is the default (last used panel), the Rules 
view is not yet instantiated. To guard against this, the Fonts panel makes a call 
to ensure an instance of the Rules view is created (and with it a CSSRuleView object).

For some reason, the pageStyle wasn't immediately assigned to the CSSRuleView in 
the constructor. The constructor signature shows that pageStyle can be passed in as a
param, but this never happens. There's only one usage of `new CSSRuleView()`.
The pageStyle exist on the inspector instance passed in to the CSSRuleView. 

This patch ensures that the CSSRuleView makes use of the PageStyleFront instance
from the inspector and removes the unused param from the constructor.

Perhaps it's better for the Fonts panel to manage its own ElementStyle instance to
get access to the element's selected rules. But in the interest of time, since the
merge date is soon, I'd rather have this fix in quikcly now and keep the dependency
to a Rules view instance with the promise to revisit the Fonts panel architecture and
remove this dependency during the Firefox 65 Nightly cycle.
Attachment #9017940 - Attachment description: Bug 1497950 - Ensure CssRuleView has immediate access to PageStyleFront from inspector; r=gl → Bug 1497950 - Ensure CSSRuleView has immediate access to PageStyleFront from inspector; r=gl
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32670a4721d4
Ensure CSSRuleView has immediate access to PageStyleFront from inspector; r=gl
https://hg.mozilla.org/mozilla-central/rev/32670a4721d4
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Keywords: regression
Tracking as a potential ride-along if we have a dot release.
The issue was verified as fixed on Firefox Nightly 64.0a1 (2018-10-22) and Firefox Beta 64.0b3 under Windows 10 (x64), macOS 10.13 and Ubuntu 16.04.
Status: RESOLVED → VERIFIED
The patch is minimal and baked 2 weeks on nightly, Razvan do you want to request an uplift to mozilla-release for inclusion in a dot release?
Flags: needinfo?(rcaliman)
Thanks, Pascal! 
The issue was only occurring in Firefox 64 for which the uplift was successful. 
I tested in the current stable, Firefox 63.0.1, which doesn't have the uplift, and the issue does not exist. 

So I see no need to uplift this patch in a dot release for stable.
Flags: needinfo?(rcaliman)
Thanks, adjusting flags accordingly.
You need to log in before you can comment on or make changes to this bug.