Closed Bug 981041 Opened 6 years ago Closed 5 years ago

Add back preference to disable transitions in Style Editor

Categories

(DevTools :: Style Editor, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 34

People

(Reporter: harth, Assigned: sayan.chowdhury2012, Mentored)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 1 obsolete file)

We had a pref to disable the CSS transitions when the Style Editor changes a style on the page.

It was unused, but we should add it back so we can disable the transitions for tests (see the orange at bug 966805).
This involves adding a new pref "pref("devtools.styleeditor.transitions", true)". And setting this pref to `false` for the test at browser/devtools/styleeditor/test/browser_styleeditor_sourcemap_watching.js.
Whiteboard: [good-first-bug][mentor="fayearthur@gmail.com"]
The code for disabling the transition would go in browser/devtools/styleeditor/StyleSheetEditor.jsm in the "_updateStyleSheet" function, the second argument to stylesheet.update() is whether or not to use transitions (right now it's always true).
I would like to work on this bug.

Thanks,
Sayan
Assignee: nobody → sayan.chowdhury2012
(In reply to sayan.chowdhury2012 from comment #3)
> I would like to work on this bug.
> 
> Thanks,
> Sayan

Hi Sayan, let me know if you can work on this. I can do it if you don't have time, no problem.
Flags: needinfo?(sayan.chowdhury2012)
Hi,

Sorry for being late, took me sometime reading stuffs. 

I want to know whenever i set "devtools.styleeditor.transitions" to false via browser, I get an error "this._notifyStyleApplied is not a function". I tried search for the function but did not get. How to solve this error?

Also, I want to know do I need to create the browser_styleeditor_sourcemap_watching.js file as it does not exist.
Flags: needinfo?(sayan.chowdhury2012)
(In reply to sayan.chowdhury2012 from comment #5)
> I want to know whenever i set "devtools.styleeditor.transitions" to false
> via browser, I get an error "this._notifyStyleApplied is not a function". I
> tried search for the function but did not get. How to solve this error?

Hm, I'm not sure why you're getting this error, if you post your current patch, I could take a look though.

> Also, I want to know do I need to create the
> browser_styleeditor_sourcemap_watching.js file as it does not exist.

Ah, this file is at browser/devtools/styleeditor/test/browser_styleeditor_sourcemap_watching.js
Attached patch 981041.patch (obsolete) — Splinter Review
I have the added the patch that was giving this particular "TypeError: this._notifyStyleApplied is not a function"

> Ah, this file is at
> browser/devtools/styleeditor/test/browser_styleeditor_sourcemap_watching.js

Ok, I got the file. Thanks
(In reply to sayan.chowdhury2012 from comment #7)
> Created attachment 8402163 [details] [diff] [review]
> 981041.patch
> 
> I have the added the patch that was giving this particular "TypeError:
> this._notifyStyleApplied is not a function"

That's coming from toolkit/devtools/server/actors/stylesheets.js
> That's coming from toolkit/devtools/server/actors/stylesheets.js

Yes, the error is coming from that file. The method notifyStyleApplied is not defined, so should i define that function?
Whiteboard: [good-first-bug][mentor="fayearthur@gmail.com"] → [good first bug][mentor="fayearthur@gmail.com"]
Mentor: fayearthur
Whiteboard: [good first bug][mentor="fayearthur@gmail.com"] → [good first bug]
Blocks: 966805
Blocks: 1038511
Here's a rebased patch.

Thanks Sayan for the patch, it looks good. I figured out that issue and made the minor fix. I'm sorry for taking so long to get back around to this.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=91d4f35e32c2
Attachment #8402163 - Attachment is obsolete: true
Attachment #8460709 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/d3196bd29f1a
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d3196bd29f1a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][fixed-in-fx-team] → [good first bug]
Target Milestone: --- → Firefox 34
QA Whiteboard: [qa+]
(In reply to Heather Arthur  [:harth] from comment #0)
> We had a pref to disable the CSS transitions when the Style Editor changes a
> style on the page.
> 
> It was unused, but we should add it back so we can disable the transitions
> for tests (see the orange at bug 966805).

Since this is for our tests (and thus also covered by them) I don't see a reason for additional testing from the Manual QA team. Setting as qe-verify-. Set it back to qe-verify+ if you think it needs the attention of the QA team.
QA Whiteboard: [qa+]
Flags: qe-verify-
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.