Closed Bug 1112947 Opened 10 years ago Closed 9 years ago

Intermittent browser_pdfjs_navigation.js | Outline items have ben found | Document is left on the last page - Got 1, expected 5

Categories

(Firefox :: PDF Viewer, defect)

Product:
Firefox
Component:
PDF Viewer
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 40
Tracking Flags:
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: RyanVM, Assigned: Snuffleupagus)

References

Details

(Keywords: intermittent-failure, Whiteboard: [pdfjs-c-integration])

Attachments

(1 file)

bug-1112947.patch
3.33 KB, patch
mossop
: review+
Details | Diff | Splinter Review 
14:11:48 INFO - 4 INFO TEST-START | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js
14:11:49 INFO - -*-*- UserCustomizations (parent): document created: http://example.com/browser/browser/extensions/pdfjs/test/file_pdfjs_test.pdf
14:11:49 INFO - -*-*- UserCustomizations (parent): _injectInWindow
14:11:49 INFO - -*-*- UserCustomizations (parent): principal status: 0
14:11:52 INFO - ###!!! [Parent][OnMaybeDequeueOne] Error: Channel closing: too late to send/recv, messages will be lost
14:12:14 INFO - 5 INFO checking window state
14:12:15 INFO - 6 INFO TEST-PASS | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | pdf handler defaults to always-ask is false
14:12:15 INFO - 7 INFO TEST-PASS | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | pdf handler defaults to internal
14:12:15 INFO - 8 INFO Pref action: 3
14:12:15 INFO - 9 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'fullscreen'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 101 column: 1 source: ":fullscreen .pdfViewer .page {"}]
14:12:15 INFO - 10 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'fullscreen'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 194 column: 17 source: "#viewerContainer:fullscreen {"}]
14:12:15 INFO - 11 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'fullscreen'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 209 column: 1 source: ":fullscreen a:not(.internalLink) {"}]
14:12:15 INFO - 12 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'fullscreen'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 217 column: 1 source: ":fullscreen .textLayer > div {"}]
14:12:15 INFO - 13 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'selection'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 1307 column: 2 source: "::selection { background: rgba(0,0,255,0.3); }"}]
14:12:15 INFO - 14 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'selection'. Ruleset ignored due to bad selector." {file: "resource://pdf.js/web/viewer.css" line: 1310 column: 13 source: ".textLayer ::selection { background: rgb(0,0,255); }"}]
14:12:15 INFO - 15 INFO TEST-PASS | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | document content has viewer UI
14:12:15 INFO - 16 INFO TEST-PASS | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | window content has PDFJS object
14:12:15 INFO - 17 INFO TEST-UNEXPECTED-FAIL | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | Outline items have ben found -
14:12:15 INFO - Stack trace:
14:12:15 INFO - chrome://mochitests/content/browser/browser/extensions/pdfjs/test/browser_pdfjs_navigation.js:runTests/<:196
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Handler.prototype.process:873
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.walkerLoop:749
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.scheduleWalkerLoop/<:691
14:12:15 INFO - null:null:0
14:12:15 INFO - 18 INFO TEST-UNEXPECTED-FAIL | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | Document is left on the last page - Got 1, expected 5
14:12:15 INFO - Stack trace:
14:12:15 INFO - chrome://mochikit/content/browser-test.js:test_is:837
14:12:15 INFO - chrome://mochitests/content/browser/browser/extensions/pdfjs/test/browser_pdfjs_navigation.js:test/eventHandler/</<:173
14:12:15 INFO - chrome://mochitests/content/browser/browser/extensions/pdfjs/test/browser_pdfjs_navigation.js:runTests/<:197
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Handler.prototype.process:873
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.walkerLoop:749
14:12:15 INFO - resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.scheduleWalkerLoop/<:691
14:12:15 INFO - null:null:0
14:12:16 INFO - 19 INFO TEST-OK | browser/extensions/pdfjs/test/browser_pdfjs_navigation.js | took 28712ms
Looks like bug 1098246 caused it, but may be a problem with bug 875766.
Blocks: 1098246
Whiteboard: [pdfjs-c-integration]
That last instance is absolutely bug 1114541, a broken slave and nothing to do with you, the next to last probably is, and so it's extremely likely that all of them are. I'd totally ignore it unless it ever happens on not-Linux, if I were you.
So my patch for bug 1142669 made this perma orange on Linux until it was backed out. The test got slower due to excessive JIT compilation (I'm fixing that of course).

But the test will fail this way when the 10 second setTimeout below fires too soon. Using setTimeout in mochitests is strongly discouraged (see bug 649012) and we should probably update the test to stop doing that.

function waitForOutlineItems(document) {
  var deferred = Promise.defer();
  var timeout = setTimeout(() => deferred.reject(), 10000);
  var interval = setInterval(function () {
    if (document.querySelectorAll(".outlineItem").length == PDF_OUTLINE_ITEMS) {
      clearInterval(interval);
      clearTimeout(timeout);
      deferred.resolve();
    }
  }, 500);
Flags: needinfo?(malutan.cosmin)
:mossop, since you reviewed the patch that originally added this test (bug 875766), I'm hoping that you can review this too.

Please note: Unfortunately I don't have access to try, so I'll need help testing this (but note that it *does* pass locally).
Assignee: nobody → jonas.jenwald
Status: NEW → ASSIGNED
Flags: needinfo?(malutan.cosmin)
Attachment #8585146 - Flags: review?(dtownsend)
Depends on: 1148192
Attachment #8585146 - Flags: review?(dtownsend) → review+
:mossop, Thanks for the review!

Could you please push the patch to Try for me?
Flags: needinfo?(dtownsend)
Keywords: checkin-needed
(In reply to Dave Townsend [:mossop] from comment #23)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=67db6aef02f1

Is the bc3 test bustage realted to this push ?
Flags: needinfo?(jonas.jenwald)
Keywords: checkin-needed
(In reply to Carsten Book [:Tomcat] from comment #24)
> (In reply to Dave Townsend [:mossop] from comment #23)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=67db6aef02f1
> 
> Is the bc3 test bustage realted to this push ?

I don't believe it is, since the test that this patch fixes runs in bc2 (bc1 in e10s, but it's skipped there).
Also, this line in the Try push looks suspect in the bc3 failures: https://hg.mozilla.org/try/rev/4c5fb77ce9f0#l2.13. Given that the patch doesn't even touch that file, the failures should be unrelated to the patch as far as I can tell.
Flags: needinfo?(jonas.jenwald)
When re-reading my response in comment 25 I realise that I probably didn't express myself clearly, sorry about that!

It appears that the Try push, by mistake, modified a line in a test file that the patch in this bug does *not* actually touch.

Based on the above: can the patch be checked-in as is?
Or does it require another round of testing (in which case I'd need help with that, since I don't have Try access)?
Flags: needinfo?(cbook)
Sorry looks like I must have had some other changes in my tree when I pushed this to try. I think you can safely ignore the bc3 failures.
Based on comment 27, I'm re-adding the checkin-needed flag.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/6ae5581862be
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [pdfjs-c-integration] → [pdfjs-c-integration][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/6ae5581862be
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Whiteboard: [pdfjs-c-integration][fixed-in-fx-team] → [pdfjs-c-integration]
Target Milestone: --- → Firefox 40
Jonas tells me that landing this on 38 was a bad idea.
https://hg.mozilla.org/releases/mozilla-release/rev/8fc6195511e5
status-firefox38: fixed → wontfix
Flags: needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: