Closed Bug 870004 Opened 7 years ago Closed 6 years ago

"Remotable Style Editor" broke stylesheet links from the Inspector

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(firefox22 unaffected, firefox23+ fixed, firefox24 fixed)

RESOLVED FIXED
Firefox 24
Tracking Status
firefox22 --- unaffected
firefox23 + fixed
firefox24 --- fixed

People

(Reporter: dcrewi, Assigned: dcrewi)

References

Details

(Keywords: regression, Whiteboard: [fixed-in-fx-team])

Attachments

(1 file)

Bug 816967 (remotable style editor) changed the API of StyleEditorPanel#selectStyleSheet, but didn't change the calls to it in the inspector. I am a little surprised that a test didn't catch this.
Assignee: nobody → dcrewi
Status: NEW → UNCONFIRMED
Ever confirmed: false
Oooh. There are two tests for this: browser_computedview_734259_style_editor_link.js and browser/devtools/styleinspector/test/browser_ruleview_734259_style_editor_link.js, but it looks like they don't test much, just that the style editor is opened.
Status: UNCONFIRMED → NEW
Ever confirmed: true
dcrew: This is a regression and would like to get this in before the merge on Monday. Any chance of a patch this weekend or should harth take it?
Status: NEW → ASSIGNED
Keywords: regression
Priority: -- → P1
Someone else can take it. The fix itself is very simple, but updating the tests to catch it next time is not as simple.
Attached patch patch v1Splinter Review
So I did end up working on this after all.
Attachment #748453 - Flags: review?(fayearthur)
Comment on attachment 748453 [details] [diff] [review]
patch v1

Review of attachment 748453 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for looking into this. This looks good. One thing I realized is that keying on the stylesheet's href doesn't work for multiple inline style sheets, as they all have an href of undefined. We might have to key off of their styleSheetIndex.

If that's an easy fix, update the patch. Otherwise, this is certainly better than broken.
I'm going to check this in, and barring any backouts, file a follow up bug about the inline stylesheets.
Attachment #748453 - Flags: review?(fayearthur) → review+
https://hg.mozilla.org/mozilla-central/rev/b906018611c8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
This should probably be pushed to firefox 23, given that it's a regression. How does one go about doing that?
I was going to do an aurora-approval request but then I realized that this patch can't be applied cleanly to the Aurora codebase. Somebody (David? Heather?) will need to check out mozilla-aurora, rebase the patch, get r+ and then file a?

I (or Heather) can check it in directly to the Aurora channel after that.

Also, reopening the bug since we're tracking Firefox 23 now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 748453 [details] [diff] [review]
patch v1

This patch applies for me on the latest from http://hg.mozilla.org/releases/mozilla-aurora/

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 816967

User impact if declined:
Clicking on a link in the the Style Inspector would open a blank Style Editor with no sheet selected, clearly broken behavior.

Testing completed (on m-c, etc.):
In m-c

Risk to taking this patch (and alternatives if risky): 
Low risk, only touches devtools code.

String or IDL/UUID changes made by this patch:
None
Attachment #748453 - Flags: approval-mozilla-aurora?
Attachment #748453 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed to Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/bca5f2d86152
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
FYI, we typically don't reopen bugs for branch uplifts. The bug resolution is used to track when a patch lands on mozilla-central. The tracking flags (like I just set for you here) are for branch uplifts.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.