Port |Bug 595652 - fix hit testing for border-radius| to SeaMonkey

VERIFIED FIXED in seamonkey2.1b3

Status

SeaMonkey
Tabbed Browser
VERIFIED FIXED
7 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 4 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

7 years ago
Created attachment 508353 [details] [diff] [review]
(Av1-WIP1) Just copy it

Switching the last occurrence to synthesizeMouseAtCenter(), instead of '9, 30' causes
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/br
owser_bug462289.js | tab not focused via middle click - Didn't expect [object XU
LElement], but got it
}
:-<
Attachment #508353 - Flags: feedback?(neil)
(Assignee)

Updated

7 years ago
Attachment #508353 - Flags: feedback?(iann_bugzilla)

Comment 2

7 years ago
Comment on attachment 508353 [details] [diff] [review]
(Av1-WIP1) Just copy it

Not sure what that's trying to test - tab2 was the activeElement and presumably therefore the selected tab and is thus always focusable.

Comment 3

7 years ago
(In reply to comment #2)
> Comment on attachment 508353 [details] [diff] [review]
> (Av1-WIP1) Just copy it
> 
> Not sure what that's trying to test - tab2 was the activeElement and presumably
> therefore the selected tab and is thus always focusable.

Doesn't middle click on a tab do different things for FF and SM?
For FF it closes the tab you're middle clicking on, for SM it tries to load what is in the copy/paste buffer as a URL

Comment 4

7 years ago
Comment on attachment 508353 [details] [diff] [review]
(Av1-WIP1) Just copy it

>-  EventUtils.synthesizeMouse(tab2, 9, 30, {button: 1, type: "mousedown"});
>+//  EventUtils.synthesizeMouse(tab2, 9, 30, {button: 1, type: "mousedown"});
>+  EventUtils.synthesizeMouseAtCenter(tab2, {button: 1, type: "mousedown"});
Remove the commented out line.

> function step6()
> {
>-  isnot(document.activeElement, tab2, "tab not focused via middle click");
>+  todo_isnot(document.activeElement, tab2, "tab not focused via middle click");
>+//  isnot(document.activeElement, tab2, "tab not focused via middle click");
As we don't seem to do the same thing for middle click as FF, what we get should be different, so the test/result should be amended - different button maybe?
Attachment #508353 - Flags: feedback?(iann_bugzilla) → feedback-
(Assignee)

Comment 5

7 years ago
(In reply to comment #2)
> Not sure what that's trying to test - tab2 was the activeElement and presumably
> therefore the selected tab and is thus always focusable.

I tried to reproduce this test manually in both FF and SM.

At end of step 2, there is a click on tab1 which is already active;
on FF, this doesn't change anything: focus remains in the urlbar;
on SM, focus moves from the urlbar to tab1 itself.
Step 3 then does |document.getElementById("urlbar").focus();| (to resync').
This test currently passes, but after adding a little delay before step 2 the activeElement check fails :->

Something similar happens at step 5:
tab2 is active, step 5 does |content.focus();| then middle-click tab2.
FF: unchanged; SM: focus moves to tab2 (hence the step 6 check failure).

Iiuc, either SeaMonkey should stop focusing its tabs or this test should take that "specific" behavior into account.
Neil?


(In reply to comment #3)
> Doesn't middle click on a tab do different things for FF and SM?
> For FF it closes the tab you're middle clicking on, for SM it tries to load
> what is in the copy/paste buffer as a URL

Not on my Windows 2000:
new profiles, FF and SM both close the tab I'm middle-clicking on (at the center or not).

Moreover, step 5 sends a "mousedown" only, no "mouseup" so no tab closing.
(Assignee)

Comment 6

7 years ago
Created attachment 511962 [details] [diff] [review]
(Bv1-FF) Add little delays to be surer, Close tabs in reverse order

This doesn't change anything atm: it just makes sure not to miss a comment 5 -like issue. (Hoping 25ms is enough in "all" cases...)
Attachment #511962 - Flags: review?(enndeakin)
(Assignee)

Comment 7

7 years ago
(In reply to comment #5)
> Iiuc, either SeaMonkey should stop focusing its tabs or this test should take
> that "specific" behavior into account.
> Neil?

Ftr, (FF) |Bug 462289 - Clicking the selected tab should not focus the tab bar|.
Blocks: 462289
(Assignee)

Comment 8

7 years ago
(In reply to comment #1)
> Switching the last occurrence to synthesizeMouseAtCenter(), instead of '9, 30'

Fwiw, I still can't figure out why step 6 check always passes with '9, 30' :-<

Comment 9

7 years ago
(In reply to comment #7)
> (In reply to comment #5)
> > Iiuc, either SeaMonkey should stop focusing its tabs or this test should take
> > that "specific" behavior into account.
> > Neil?
> Ftr, (FF) |Bug 462289 - Clicking the selected tab should not focus the tab
> bar|.
No, I don't want to port that.
(Assignee)

Comment 10

7 years ago
Created attachment 511993 [details] [diff] [review]
(Bv2-FF) Add little delays to be surer, Add/Update some checks, Fix some nits

Av1-FF, enhanced.

You may want to investigate/fix my 'XXX' comment based on observed behavior on my Windows 2000.
Attachment #511962 - Attachment is obsolete: true
Attachment #511993 - Flags: review?(enndeakin)
Attachment #511962 - Flags: review?(enndeakin)
(Assignee)

Updated

7 years ago
Attachment #511962 - Attachment description: (Av1-FF) Add little delays to be surer, Close tabs in reverse order → (Bv1-FF) Add little delays to be surer, Close tabs in reverse order
Attachment #511962 - Attachment filename: 630140-Av1-FF_delays.diff → 630140-Bv1-FF_delays.diff
(Assignee)

Updated

7 years ago
Attachment #511993 - Attachment description: (Av2-FF) Add little delays to be surer, Add/Update some checks, Fix some nits → (Bv2-FF) Add little delays to be surer, Add/Update some checks, Fix some nits
Attachment #511993 - Attachment filename: 630140-Av2-FF_delays.diff → 630140-Bv2-FF_delays.diff
(Assignee)

Comment 11

7 years ago
Created attachment 511994 [details] [diff] [review]
(Av2-SM) Port bug 595652, Add needed little delays, Add/Update some checks, Fix some nits
[Checked in: Comment 20]

Av1-WIP1, sync'ed with Bv2-FF,
taking SMvsFF differences.
Attachment #508353 - Attachment is obsolete: true
Attachment #511994 - Flags: review?(iann_bugzilla)
Attachment #508353 - Flags: feedback?(neil)
(Assignee)

Comment 12

7 years ago
Created attachment 512020 [details] [diff] [review]
(Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too
[Checked in: Comment 17]

(Nit) Fixes to bug 597584 and bug 599745.
Attachment #512020 - Flags: review?(neil)
Attachment #512020 - Flags: review?(iann_bugzilla)
Attachment #512020 - Flags: feedback?(misak.bugzilla)
Attachment #512020 - Flags: feedback?(aqualon)

Comment 13

7 years ago
Comment on attachment 512020 [details] [diff] [review]
(Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too
[Checked in: Comment 17]

>-    gBrowser.selectedTab  = gBrowser.addTab("about:blank", {skipAnimation: true});
>+    // Ftr, SeaMonkey doesn't support animation (yet).
>+    gBrowser.selectedTab  = gBrowser.addTab("about:blank");
> 

Neil asks me to use getBrowser() instead of gBrowser, as our gBrowser is a little bit different.
Attachment #512020 - Flags: feedback?(misak.bugzilla) → feedback+
Does executeSoon work better that having to use timeouts?

Updated

6 years ago
Attachment #512020 - Flags: review?(neil) → review+

Updated

6 years ago
Attachment #512020 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 15

6 years ago
(In reply to comment #14)
> Does executeSoon work better that having to use timeouts?

I don't think so in this file: the goal is not (only) to complete running the current test before something else runs, but rather to do "ensure" every(possible)thing has run before the next test starts.
(Assignee)

Comment 16

6 years ago
(In reply to comment #15)
> (In reply to comment #14)
> > Does executeSoon work better that having to use timeouts?
> 
> I don't think so in this file

As least, executeSoon() is no better than setTimeout(,0) with SeaMonkey on my Windows wrt comment 5.
(Assignee)

Comment 17

6 years ago
Comment on attachment 512020 [details] [diff] [review]
(Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too
[Checked in: Comment 17]

http://hg.mozilla.org/comm-central/rev/f06a82f647b9
Attachment #512020 - Attachment description: (Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too → (Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too [Checked in: Comment 17]
(Assignee)

Comment 18

6 years ago
(In reply to comment #13)
> Neil asks me to use getBrowser() instead of gBrowser, as our gBrowser is a
> little bit different.

If we do want to do that, I assume that would deserve its own bug:
http://mxr.mozilla.org/comm-central/search?string=%5B%5Ea-z%5DgBrowser%5B%5EA-Z%5D&regexp=1&case=1&find=%2Fsuite%2F
"Found 315 matching lines in 42 files"

Comment 19

6 years ago
Comment on attachment 511994 [details] [diff] [review]
(Av2-SM) Port bug 595652, Add needed little delays, Add/Update some checks, Fix some nits
[Checked in: Comment 20]

>-  tab1 = gBrowser.addTab();
>-  tab2 = gBrowser.addTab();
>+  // Ftr, SeaMonkey doesn't support animation (yet).
>+  tab1 = gBrowser.addTab("about:blank");
>+  tab2 = gBrowser.addTab("about:blank");
The behaviour is the same for no argument as it is for "about:blank", so no real need to change it.

r=me with either.
Attachment #511994 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 20

6 years ago
Comment on attachment 511994 [details] [diff] [review]
(Av2-SM) Port bug 595652, Add needed little delays, Add/Update some checks, Fix some nits
[Checked in: Comment 20]

http://hg.mozilla.org/comm-central/rev/c8ef067543ad


(In reply to comment #19)
> The behaviour is the same for no argument as it is for "about:blank", so no
> real need to change it.

I know: I preferred to keep code closer to FF test.
Attachment #511994 - Attachment description: (Av2-SM) Port bug 595652, Add needed little delays, Add/Update some checks, Fix some nits → (Av2-SM) Port bug 595652, Add needed little delays, Add/Update some checks, Fix some nits [Checked in: Comment 20]
(Assignee)

Updated

6 years ago
Blocks: 597584, 599745
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
(Assignee)

Comment 21

6 years ago
Linux and Windows (at least) report
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_bug462289.js | 2nd click on selected tab1 activates tab - Got [object XULElement @ 0x9bdda00 (native @ 0x9afe460)], expected [object XULElement @ 0x9be3f88 (native @ 0x9608ca0)]
}
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 511993 [details] [diff] [review]
(Bv2-FF) Add little delays to be surer, Add/Update some checks, Fix some nits

>-  setTimeout(step2, 0);
>+  // Here and after, don't use a delay of '0': give a little more time to be sure UI has fully updated.
>+  setTimeout(step2, 25);

What problem are you trying to solve here exactly?
(Assignee)

Comment 23

6 years ago
Created attachment 515598 [details] [diff] [review]
(Dv1-SM) Increase delay to 250 ms from 25
[Checked in: See comment 30+31]

+/- expected: I guess our Dbg slaves are slower than my Opt Windows 2000.

Ftr, 10 was not enough when I tested locally.
Attachment #515598 - Flags: review?(iann_bugzilla)
Comment on attachment 511993 [details] [diff] [review]
(Bv2-FF) Add little delays to be surer, Add/Update some checks, Fix some nits

Looks like SeaMonkey has a problem with the zero-timeout that Firefox doesn't have. I don't see why the test for Firefox should accommodate this.
Attachment #511993 - Flags: review?(enndeakin) → review-
(Assignee)

Comment 25

6 years ago
(In reply to comment #22)
> What problem are you trying to solve here exactly?

This test checks where focus is,
but, from what I observed with SeaMonkey, a delay of '0' is not enough to let UI fully update,
hence FF (would) always succeeds and SM always fails, which is a broken check for both applications.

(In reply to comment #24)
> Looks like SeaMonkey has a problem with the zero-timeout that Firefox doesn't
> have. I don't see why the test for Firefox should accommodate this.

I assumed the UI update "synchronization" issue was in the test, not in SeaMonkey, so Firefox test should be updated too.
Are you saying that Firefox test with current setTimeout(0) would be able to catch a focus regression?
If yes, would you have any idea what makes UI update different on SeaMonkey?
(In reply to comment #25)
> Are you saying that Firefox test with current setTimeout(0) would be able to
> catch a focus regression?

I wouldn't rule it out. Hard to tell without knowing what's currently going on with SeaMonkey.

> If yes, would you have any idea what makes UI update different on SeaMonkey?

No idea.
(In reply to comment #21)
> Linux and Windows (at least) report
> {
> TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/browser/suite/browser/test/browser_bug462289.js |
> 2nd click on selected tab1 activates tab - Got [object XULElement @ 0x9bdda00
> (native @ 0x9afe460)], expected [object XULElement @ 0x9be3f88 (native @
> 0x9608ca0)]
> }

before I am happy to take a simple timeout change here, what changed in m-c (*or* us) during this timeframe from "passing" to "failing".
(Assignee)

Comment 28

6 years ago
(In reply to comment #27)
> what changed in m-c
> (*or* us) during this timeframe from "passing" to "failing".

Nothing changed: this check has been failing since the changeset landed.

Comment 29

6 years ago
Comment on attachment 515598 [details] [diff] [review]
(Dv1-SM) Increase delay to 250 ms from 25
[Checked in: See comment 30+31]

This does seem a big jump, could we start smaller (100 maybe) and increase until we get a constant green?
r=me with that answered/fixed.
Attachment #515598 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 30

6 years ago
Comment on attachment 515598 [details] [diff] [review]
(Dv1-SM) Increase delay to 250 ms from 25
[Checked in: See comment 30+31]

http://hg.mozilla.org/comm-central/rev/2bb4e4f86e8d
Dv1, with comment 29 suggestion(s).
Attachment #515598 - Attachment description: (Dv1-SM) Increase delay to 250 ms from 25 → (Dv1-SM) Increase delay to 250 ms from 25 [Checked in: See comment 30]
(Assignee)

Comment 31

6 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1299592533.1299595426.21028.gz
Linux comm-central-trunk debug test mochitest-other on 2011/03/08 05:55:33
still failed.

***

http://hg.mozilla.org/comm-central/rev/22e2a5cf34ab
(Ev1-SM) Increase delay to 250 ms from 100

Let's try to turn green first...
(Assignee)

Updated

6 years ago
Attachment #515598 - Attachment description: (Dv1-SM) Increase delay to 250 ms from 25 [Checked in: See comment 30] → (Dv1-SM) Increase delay to 250 ms from 25 [Checked in: See comment 30+31]
(Assignee)

Comment 32

6 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1299608236.1299610701.19157.gz
Linux comm-central-trunk debug test mochitest-other on 2011/03/08 10:17:16
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1299610302.1299613895.32705.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2011/03/08 10:51:42

V.Fixed
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Status: RESOLVED → VERIFIED
(Assignee)

Comment 33

6 years ago
Created attachment 517902 [details] [diff] [review]
(Bv3-FF) Add/Update some checks, Fix some nits
[Moved to bug 706149]

Bv2-FF, without increased delay :-|
Attachment #511993 - Attachment is obsolete: true
Attachment #517902 - Flags: review?(dao)
(Assignee)

Comment 34

6 years ago
2 more green Linux runs, but 1 orange Windows run:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1299622063.1299626155.16075.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2011/03/08 14:07:43

I'll increase the delay some more if I notice this random-orange (too) often...
(Assignee)

Comment 35

6 years ago
Windows orange happens quite often.

http://hg.mozilla.org/comm-central/rev/06d9e044fd7a
(Fv1-SM) Increase delay to 375 ms from 250

Comment 36

6 years ago
Comment on attachment 517902 [details] [diff] [review]
(Bv3-FF) Add/Update some checks, Fix some nits
[Moved to bug 706149]

Removing review request to take this bug off SeaMonkey bug review queries. Please file a separate bug in the Firefox component.
Attachment #517902 - Flags: review?(dao)

Comment 37

6 years ago
Comment on attachment 512020 [details] [diff] [review]
(Cv1-SM) Remove unsupported tab 'skipAnimation' param in other tests too
[Checked in: Comment 17]

Cancelling feedback request since patch has been checked in months ago.
Attachment #512020 - Flags: feedback?(aqualon)
(Assignee)

Updated

6 years ago
Blocks: 706149
(Assignee)

Updated

6 years ago
Attachment #517902 - Attachment description: (Bv3-FF) Add/Update some checks, Fix some nits → (Bv3-FF) Add/Update some checks, Fix some nits [Moved to bug 706149]
Attachment #517902 - Attachment is obsolete: true

Updated

5 years ago
Blocks: 779720

Updated

5 years ago
No longer blocks: 779720
You need to log in before you can comment on or make changes to this bug.