Closed Bug 731948 Opened 8 years ago Closed 8 years ago

We should have a waitFor in the setter of selectedIndex

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: remus.pop, Assigned: remus.pop)

References

Details

(Whiteboard: [lib])

Attachments

(1 file)

Currently the setter of selecteIndex(index) clicks the tab, but does not wait for the action to happen.

Current code is:
set selectedIndex(index) {
    this._controller.click(this.getTab(index));
  }
So my proposal would be
set selectedIndex(index) {
    this._controller.waitFor(function (){
      this._controller.click(this.getTab(index));
      return this.selectedIndex === index;
    }, "The tab with index '" + i + "' has been selected");
  }
meaning that we should click until that tab will be selected.
Henrik is this solution ok? Otherwise we will only click once and then wait forever until the tab is selected, which of course is never going to happen.
Why is it never going to happen? We should never flood Firefox with clicks on elements in a waitFor call. So move it out of the callback.
Because we click it once. And if the tabs are not displayed yet, it would click nowhere. It doesn't fail all the time with your idea, but sometimes.
Going against the flood of clicks like you said, maybe we should tweak the interval to 500, for example. How does that sound?
Blocks: 731155
Then the failure is somewhere else. Are you talking about transition out of the private browsing mode? If yes, then we should fix it there. Otherwise as I said, click once.
Yes, it was about that transition, but a waitFor here won't hurt at all. So this bug will go with clicking once an waiting for the tab.
Right. That's how it should be implemented. Thanks Remus.
Not sure why, but this.selectedIndex is undefined and our idea fails. Don't know how it worked earlier, but in a different repo.
You will have to pass in this as last parameter for the waitFor method.
Added a waitFor call after clicking on a tab.
Attachment #602293 - Flags: review?(vlad.mozbugs)
Attachment #602293 - Attachment description: patch v1 → patch v1 (all branches)
Comment on attachment 602293 [details] [diff] [review]
patch v1 (all branches)

+ from me, Henrik's input is important here. 
Over to him for sr
Attachment #602293 - Flags: review?(vlad.mozbugs)
Attachment #602293 - Flags: review?(hskupin)
Attachment #602293 - Flags: review+
Comment on attachment 602293 [details] [diff] [review]
patch v1 (all branches)

Looks good to me.
Attachment #602293 - Flags: review?(hskupin) → review+
Would be good if someone else could check it in. Thanks.
Keywords: checkin-needed
Landed on default as:
http://hg.mozilla.org/qa/mozmill-tests/rev/22b9f028868d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Component: Mozmill Shared Modules → Mozmill Tests
Whiteboard: [lib]
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.