Closed
Bug 630140
Opened 13 years ago
Closed 13 years ago
Port |Bug 595652 - fix hit testing for border-radius| to SeaMonkey
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
VERIFIED
FIXED
seamonkey2.1b3
People
(Reporter: sgautherie, Assigned: sgautherie)
References
()
Details
Attachments
(3 files, 4 obsolete files)
4.14 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
2.25 KB,
patch
|
neil
:
review+
iannbugzilla
:
review+
misak.bugzilla
:
feedback+
|
Details | Diff | Splinter Review |
3.24 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
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•13 years ago
|
Attachment #508353 -
Flags: feedback?(iann_bugzilla)
Comment 2•13 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.
(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 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•13 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•13 years ago
|
||
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•13 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•13 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•13 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•13 years ago
|
||
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•13 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•13 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•13 years ago
|
||
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•13 years ago
|
||
(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•13 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+
Comment 14•13 years ago
|
||
Does executeSoon work better that having to use timeouts?
Updated•13 years ago
|
Attachment #512020 -
Flags: review?(neil) → review+
Attachment #512020 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 15•13 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•13 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•13 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•13 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®exp=1&case=1&find=%2Fsuite%2F "Found 315 matching lines in 42 files"
Comment 19•13 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•13 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•13 years ago
|
Assignee | ||
Comment 21•13 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 22•13 years ago
|
||
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•13 years ago
|
||
+/- 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 24•13 years ago
|
||
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•13 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?
Comment 26•13 years ago
|
||
(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.
Comment 27•13 years ago
|
||
(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•13 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•13 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•13 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•13 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•13 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•13 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
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 33•13 years ago
|
||
Bv2-FF, without increased delay :-|
Attachment #511993 -
Attachment is obsolete: true
Attachment #517902 -
Flags: review?(dao)
Assignee | ||
Comment 34•13 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•13 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•13 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•13 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•13 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
You need to log in
before you can comment on or make changes to this bug.
Description
•