981041 - Add back preference to disable transitions in Style Editor

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Heather Arthur [:harth]]

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).

Comment 1

[Heather Arthur [:harth]]

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.

Comment 2

[Heather Arthur [:harth]]

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).

Comment 3

[sayan.chowdhury2012]

I would like to work on this bug.

Thanks,
Sayan

Comment 4

[Heather Arthur [:harth]]

(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.

Comment 5

[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.

Comment 6

[Heather Arthur [:harth]]

(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

Comment 7

[sayan.chowdhury2012]

Attachment: 981041.patch

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

Comment 8

[Heather Arthur [:harth]]

(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

Comment 9

[sayan.chowdhury2012]

> 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?

Comment 10

[Heather Arthur [:harth]]

Attachment: Rebased + bug fix

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

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/d3196bd29f1a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d3196bd29f1a

Comment 13

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(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.