Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

issue6013: add FBTest #67

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

fflorent commented Dec 2, 2012

http://code.google.com/p/fbug/issues/detail?id=6013#23

Should work for both Simon's (comment 20) and my (comment 23) fix proposals.

@simonlindholm simonlindholm commented on the diff Dec 22, 2012

tests/FBTest/content/FBTestFirebug.js
@@ -2691,8 +2697,7 @@ function createCanvas(width, height)
// Inspector
this.inspectElement = function(elt)
{
- FBTest.clickToolbarButton(FW.Firebug.chrome, "fbInspectButton");
- FBTest.click(elt);
+ FW.Firebug.Inspector.highlightObject(elt, FW.Firebug.currentContext);
@simonlindholm

simonlindholm Dec 22, 2012

Member

Wait, shouldn't FBTest.inspectElement change the selected node? (Inspector.highlightObject doesn't do that I hope.)
Looks a bit gross, in any case. Event simulation > calling internal functions.

Member

simonlindholm commented Dec 22, 2012

If tests pass I think you can just go ahead and merge this (I won't have any time, and we shouldn't bother Honza when he's working on important JSD2 stuff ;) ).

Owner

janodvarko commented Dec 23, 2012

If tests pass I think you can just go ahead and merge this (I won't have any time,
and we shouldn't bother Honza when he's working on important JSD2 stuff ;) ).

I am actually focusing on two things:

  1. tests
  2. JSD2

So, this is actually in the area of my interest :-)


I agree with Simon that we should avoid calling internal functions and rather base the logic on events.

Honza

Member

fflorent commented Dec 23, 2012

OK. I'll propose something else once I have some time.

Merry Christmas :) !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment