Closed
Bug 1255180
Opened 8 years ago
Closed 8 years ago
Convert test_bug370436.html to work with e10s
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: dholbert, Assigned: dholbert)
References
(Blocks 2 open bugs)
Details
Attachments
(3 files)
Our mochitest-chrome tests are all disabled under e10s. This test... http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/chrome/test_bug370436.html?force=1 ...seems like something that we should be testing with e10s enabled, though. It's testing text-selection & context-menu-spawning, I think, which seems like it could differ between e10s & non-e10s browser configurations.
Assignee | ||
Comment 1•8 years ago
|
||
Handy e10s conversion doc: https://wiki.mozilla.org/Electrolysis/e10s_test_tips There are some details about synthesizeKey & synthesizeMouse there. (This test uses both of those APIs.)
Updated•8 years ago
|
Blocks: e10s-tests
tracking-e10s:
--- → +
Assignee | ||
Comment 2•8 years ago
|
||
So the first main part of the test that's chrome-only is here: > 28 function onContextMenu(e) > 29 { > 30 var node = e.rangeParent; > [...] > 35 var word = expandStringOffsetToWord(node.data, offset); "e.rangeParent", for our UIEvent "e", only returns something non-null if we're running with Chrome privileges. The check for that happens here: > 226 already_AddRefed<nsINode> > 227 UIEvent::GetRangeParent() > [...] > 240 if (parent->ChromeOnlyAccess() && > 241 !nsContentUtils::CanAccessNativeAnon()) { > 242 return nullptr; > 243 } http://mxr.mozilla.org/mozilla-central/source/dom/events/UIEvent.cpp#240 ChromeOnlyAccess() returns true because |parent| has the NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE flag (because it's some text inside of a text area), and nsContentUtils::CanAccessNativeAnon() returns false. I suspect we want to use (or add) a SpecialPowers API that will allow this code to succeed... If I make a quick local hack to simply disable the security checks in GetRangeParent(), then we successfully get a non-null "node", but we fail on the attempt to read "node.data". (Error: Permission denied to access property "data") Not yet sure where that check happens or how to get around that.
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #2) > I suspect we want to use (or add) a SpecialPowers API that will allow this > code to succeed... mccr8 tells me in that "SpecialPowers.wrap()" is what we want here. Patches coming shortly.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → dholbert
Assignee | ||
Comment 4•8 years ago
|
||
This patch also temporarily disables the test by commenting it out in mochitest.ini, its new home. (I'm disabling it temporarily because it won't pass until later patches in this queue have landed.) Review commit: https://reviewboard.mozilla.org/r/47297/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47297/
Attachment #8742565 -
Flags: review?(bobbyholley)
Attachment #8742566 -
Flags: review?(bobbyholley)
Attachment #8742567 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47299/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47299/
Assignee | ||
Comment 6•8 years ago
|
||
This patch also enables the test by uncommenting it in mochitest.ini, since it should now run successfully. Review commit: https://reviewboard.mozilla.org/r/47301/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47301/
Assignee | ||
Comment 7•8 years ago
|
||
NOTE: The only piece here that I'm not 100% sure about is the "Shouldn't have SOWs in chrome" check-removal. I'm pretty sure I should remove that, because: (1) If I convert it over so that I can run it in mochitest-plain (with this test as an e10s-enabled test) then that check fails, because the node's type is "Proxy" instead of "Text". (2) The check's text explicitly mentions "in chrome", and this test is becoming no-longer-a-chrome-mochitest. Bobby, let me know if it's important that this check stay around somehow. (You added it in Bug 832512.)
Comment 8•8 years ago
|
||
Comment on attachment 8742565 [details] MozReview Request: Bug 1255180 part 1: Move test_bug370436.html out of its mochitest-chrome subdirectory. r?bholley https://reviewboard.mozilla.org/r/47297/#review43995
Attachment #8742565 -
Flags: review?(bobbyholley) → review+
Updated•8 years ago
|
Attachment #8742566 -
Flags: review?(bobbyholley) → review+
Comment 9•8 years ago
|
||
Comment on attachment 8742566 [details] MozReview Request: Bug 1255180 part 2: Update script/style paths in test_bug370436.html to point to mochitest-plain resources. r?bholley https://reviewboard.mozilla.org/r/47299/#review43997
Comment 10•8 years ago
|
||
Comment on attachment 8742567 [details] MozReview Request: Bug 1255180 part 3: Update test_bug370436.html to use SpecialPowers for privileged "rangeParent" access, & drop now-obsolete checks for SOWs in chrome. r?bholley https://reviewboard.mozilla.org/r/47301/#review43999 We don't have SOWs anymore - they've been replaced with XBL scopes, and I'm not worried about the correctness of when we do XBL scopes.
Attachment #8742567 -
Flags: review?(bobbyholley) → review+
Assignee | ||
Comment 11•8 years ago
|
||
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=32aa3a39c9c6 It looks good, aside from unrelated mega-orange on OS X 10.10 (a platform which is hidden on trunk, presumably in part due to this mega-orange). I verified that this test is indeed showing up in M-e10s run output, too -- on Linux Opt in that Try run, it's listed in mochitest-e10s-4: > 18:59:20 INFO - 1882 INFO TEST-START | layout/base/tests/test_bug370436.html > 18:59:20 INFO - MEMORY STAT | vsize 354MB | residentFast 85MB | heapAllocated 26MB > 18:59:20 INFO - 1883 INFO TEST-OK | layout/base/tests/test_bug370436.html | took 369ms So, I'll go ahead and land. Thanks for the review!
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/02e86612b379 https://hg.mozilla.org/integration/mozilla-inbound/rev/5218444651f8 https://hg.mozilla.org/integration/mozilla-inbound/rev/d7d8092718c9
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/02e86612b379 https://hg.mozilla.org/mozilla-central/rev/5218444651f8 https://hg.mozilla.org/mozilla-central/rev/d7d8092718c9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•