Recorder and Inspector fetch incorrect document for nested frames

VERIFIED FIXED

Status

defect
VERIFIED FIXED
10 years ago
3 years ago

People

(Reporter: harth, Assigned: harth)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [verified-mozmill-1.2.4])

When you record clicking on the Jetpack slidebar, it will give you a controller for conroller.tabs.activeTab, it should be controller.window.frames[1]. Also, when you try to inspect the slidebar (or any iframe on a webpage), you get a controller with "Cannot find document".
You can also see the problem with the inspector when inspecting the DOM Inspector window. If you open the DOM Inspector, then start the Mozmill inspector and hover over a node on the DOM Inspector window without focusing the DOM Inspector window, the Element will say "Cannot find document", and the controller will be for the Mozmill IDE. And I think I have been mis-using the word 'controller', so I apologize (-:
(In reply to comment #2)
> http://github.com/harthur/mozmill/commit/aaea011fa91f0538a26361715b149e70c72b01f1

That works great for content and chrome elements! Nice to see that frames can be inspected now. For checking the code I did it mostly for the second changeset. I only have one comment which could be addressed separately though:

> case 'Browser:Preferences':

We don't need the preferences condition anymore. Those windows have to be handled via the modal dialog API. Would be nice to move it to Places:Organizer which will be the Library and we need definitely in the near future. If we wanna do that in a separate patch we can do that.

> this one is just taking out the get-window-by-type code I'd put in so we can
> test this separately:
> http://github.com/harthur/mozmill/commit/4f1511de979be4f3525c0885f158255e396e3fde

Oh, that function is fantastic. It would be great to test it together with the other changes. Sadly I cannot find the command to checkout a given changeset. :/
Assignee: nobody → harthur
Status: NEW → ASSIGNED
Summary: Recorder and Inspector fetch incorrect document for Jetpack slidebar → Recorder and Inspector fetch incorrect document for nested frames
> That works great for content and chrome elements! 

So, this brings up a small issue.

Mozmill doesn't require that the frame be specified for content windows, it traverses the frames and looks for the element in all of them.

Now, if a frame is specified we should still work, but we don't require it.

I'm wondering if there are any issues with the code generation tools (recorder & inspector) specifying the frame for content windows when it's not actually necessary. Is anyone all that worried about the test authors getting the impression that writing this stuff by hand is harder than it really is?

I don't really think it's much of a problem since you can't feasibly write a lookup expression by hand anyway, which means most testers *must* rely on the generation tools for a lot of tasks.
(In reply to comment #4)
> Mozmill doesn't require that the frame be specified for content windows, it
> traverses the frames and looks for the element in all of them.
> 
> Now, if a frame is specified we should still work, but we don't require it.

I thought about the same Mikeal but I think that this way is the correct one. Think about that frames can have elements with the same id or whatever else. Without specifying the frame how does Mozmill know which element it should use?

> I'm wondering if there are any issues with the code generation tools (recorder
> & inspector) specifying the frame for content windows when it's not actually
> necessary. Is anyone all that worried about the test authors getting the
> impression that writing this stuff by hand is harder than it really is?

They could run in trouble depending on the above mentioned situation.

> I don't really think it's much of a problem since you can't feasibly write a
> lookup expression by hand anyway, which means most testers *must* rely on the
> generation tools for a lot of tasks.

If we have frames and you don't want to have it in the first argument, we would have to always use a lookup or xpath expression, or?
(In reply to comment #5)
> I thought about the same Mikeal but I think that this way is the correct one.
> Think about that frames can have elements with the same id or whatever else.
> Without specifying the frame how does Mozmill know which element it should use?

I thought about this too, but arrived to the same conclusion - it would be incorrect otherwise, sometimes
the lookup expression is quite generic and could match on a higher-level frame before it matches on the correct frame.

I personally wouldn't try to build up the query myself unless I absolutely had to.

> If we have frames and you don't want to have it in the first argument, we would
> have to always use a lookup or xpath expression, or?

We could put this in the lookup expression, or at least allow a lookup expression in the first argument,
so you wouldn't have to depend on 'frame[3]' being the correct frame but could get a frame's document by getting
the frame element by id then getting it's contentDocument or something like that. A thought.

>We don't need the preferences condition anymore. Those windows have to be
>handled via the modal dialog API. Would be nice to move it to Places:Organizer
>which will be the Library and we need definitely in the near future. If we
>wanna do that in a separate patch we can do that.

Would you need a case for modal dialogs?

>Oh, that function is fantastic. It would be great to test it together with the
>other changes. Sadly I cannot find the command to checkout a given changeset.

There's actually one other small but important fix in that commit that you would need, so you need
that commit, I can add it back and commit again if needed.
Sorry about that formatting in my last comment, here's a more readable version:

> I thought about the same Mikeal but I think that this way is the correct one.
> Think about that frames can have elements with the same id or whatever else.
> Without specifying the frame how does Mozmill know which element it should use?

I thought about this too, but arrived to the same conclusion - it would be incorrect otherwise, sometimes the lookup expression is quite generic and could match on a higher-level frame before it matches on the correct frame.

I personally wouldn't try to build up the query myself unless I absolutely had to.

> If we have frames and you don't want to have it in the first argument, we would
> have to always use a lookup or xpath expression, or?

We could put this in the lookup expression, or at least allow a lookup expression in the first argument, so you wouldn't have to depend on 'frame[3]' being the correct frame but could get a frame's document by getting the frame element by id then getting it's contentDocument or something like that. A thought.

>We don't need the preferences condition anymore. Those windows have to be
>handled via the modal dialog API. Would be nice to move it to Places:Organizer
>which will be the Library and we need definitely in the near future. If we
>wanna do that in a separate patch we can do that.

Would you need a case for modal dialogs?

>Oh, that function is fantastic. It would be great to test it together with the
>other changes. Sadly I cannot find the command to checkout a given changeset.

There's actually one other small but important fix in that commit that you would need, so you need that commit, I can add it back and commit again if needed.
(In reply to comment #7)
> >We don't need the preferences condition anymore. Those windows have to be
> >handled via the modal dialog API. Would be nice to move it to Places:Organizer
> >which will be the Library and we need definitely in the near future. If we
> >wanna do that in a separate patch we can do that.
> 
> Would you need a case for modal dialogs?

No, thanks. We have it as a shared module:
http://hg.mozilla.org/qa/mozmill-tests/file/default/shared-modules/testModalDialogAPI.js

Once it is ready we can move it into Mozmill itself. But it will still take a while.
So this bug is fixed now? Mikeal, please add comments to the bugs when you commit a change. Otherwise we will loose track.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
That has been fixed with:
http://github.com/mikeal/mozmill/commit/aaea011fa91f0538a26361715b149e70c72b01f1

Looks great. Thanks Heather.
Status: RESOLVED → VERIFIED
Whiteboard: [verified-mozmill-1.2.4]
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.