text:
Use DropBehavior::Panic

I have some code hanging around to use this:
https://github.com/jgallagher/rusqlite/pull/325
to make it easier to spot a coding error that results in a transaction not being committed.
It might even be useful enough to be the default setting.

This will be pending on the next rusqlite release, of course.

repo: mozilla/mentat


text:
Find-in-page highlight-box is at wrong y-position in Coway airmega PDF-manual

Attach (recommended) or Link to PDF file here:
https://images-na.ssl-images-amazon.com/images/I/D1SWYNC4nwS.pdf
D1SWYNC4nwS.pdf
Configuration:

Web browser and its version: Firefox Nightly 82.0a1 (2020-09-05)
Operating system and its version: Windows 10
PDF.js version: Not sure; whatever's shipped with Nightly right now
Is a browser extension: not an extension that I manually installed; just the built-in version.

Steps to reproduce the problem:

Load attached PDF
Find-in-page (Ctrl+F) and type "clean".
(optional): Ctrl+G to skip through a few matches

What is the expected behavior?
The text "clean" should be highlighted for each match.
What went wrong? (add screenshot)
The highlighted box is above the text, approximately one line-height away:

Yeah, so I tried to reproduce the bug and it is evident that the y-position is not correct.
This is not a problem with the PDF document itself since the same document selection is rendered correctly in Edge's PDF Viewer. This probably has to do with the y-offset in rendering the selection.

Please assign this issue, I can begin some work on this.

Closing since this got fixed by #12896 and other work.

source: gharchive/issue

authors: dholbert piexpie timvandermeij

license_type: permissive

license_source: github-api


text:
Hyperlinks in PDF Rendered by vue-pdf Misaligned on Small Screens

Attach (recommended) or Link to PDF file here:
https://tcstore1.17usoft.com/tcoa-bucket/30778699df3d48a4b61d6fe335352e56.pdf
Configuration:

Web browser and its version: Google
Operating system and its version: win10
PDF.js version: vue-pdf 4.2.0 and 4.3.0

Steps to reproduce the problem:

yarn add vue-pdf

What is the expected behavior? (add screenshot)

What went wrong? (add screenshot)

Can you reproduce this opening that PDF at https://mozilla.github.io/pdf.js/web/viewer.html? If not, you should this to vue-pdf.

authors: JoyZ-nineteen nicolo-ribaudo


text:
table format in pdf is not rendered properly in browser

We tried to render a pdf using pdf.js library latest version. This pdf contains some tables with rows and columns. The partition lines of rows and columns did not appear as smooth as the native pdf reader. I have attached the screenshots for your reference

When reporting an issue, please make sure that you read https://github.com/mozilla/pdf.js/blob/master/CONTRIBUTING.md, paying special attention to the first paragraph. I'm quoting the relevant section:

[...] and provide more details about the issue: link to the PDF, [...], browser version, operating system, PDF.js version and JavaScript console warning/error messages. Issues that do not have enough details provided will be closed as invalid/incomplete.

Closing as incomplete. Please provide the information requested above to reopen.

authors: Snuffleupagus selva1980 timvandermeij


text:
Revert "Convert Catalog.getPageDict to an async method"

Reverts mozilla/pdf.js#11312, please refer to https://github.com/mozilla/pdf.js/pull/11312#issuecomment-552139930; my sincere apologies for the mess created here!

/botio test

No worries; fortunately a revert is trivial!

source: gharchive/pull-request

authors: Snuffleupagus timvandermeij


text:
Add scrolling modes to web UI

In addition to the default scrolling mode (vertical), this commit adds horizontal, grid, and grid+cover scrolling, implemented primarily with flexbox CSS.
You want some SHOTS???
The default: vertical scrolling. Yawn.

Horizontal scrolling! #2545, that's what's up!

And I know my peeps down at #590 have been begging for that GRID SCROLLING!

As you can see, weirdly-sized pages are not a problem for the mighty flexbox!
Did you want special cover page handling for your grid scrolling? I GOT YOU.

But why is it called grid scrolling instead of two-up or dual? Because depending on your zoom and your browser width, it could look like this!

Don't thank me, thank flexbox!

I manually tested this in Chrome and Firefox, with large and small PDF files, and made sure it doesn't interfere with presentation mode. I am all about adding those automated tests when submitting pull requests because that's how my momma raised me, but in this case I'm not sure what kind of test to add and/or how! If my reviewer has any ideas, I'm all ears.

Closes #590, closes #2545, opens the door to your heart.

PR has been updated to include a grid with cover page scrolling mode, and icons that are a little less janky.

Is there a way to get this as a viewer option, similar to setting up the zoom or page?  Having the ability to do scroll=Grid would be great.

/botio-linux preview

It looks like it works quite well, so nice work! Since this is quite a big viewer change, we do need some time to play with this to make sure there are no issues, but at first glance the solution looks pretty clean.
I did find one issue with horizontal scrolling so far. Open the viewer generated above, go a few pages further (for example page 6) and then select horizontal scrolling. Notice that you can scroll all the way to the right, but on the left you cannot scroll farther back than page 6, i.e., pages 1-5 are not reachable anymore.

Thanks for taking a look! I see the problem and I'll touch up the PR later tonight.

Okay, that issue should be fixed.
@jlac1024, in principle I don't see why not but what I think is the relevant code doesn't look trivially extensible. I doubt it'd be a lot of work to do, but probably someone who understands why the existing code is written the way it is should do it.

/botio-linux preview

Hi guys! Are there any news on this? Thanks!

New code! Get excited!
First, the bad news: I couldn't find a pure CSS way to keep page spreads together with all the properties I wanted, so this does back down a little bit on my claim that it's all just CSS. Since visiting @Snuffleupagus's points already involved a heavier touch on the JS side, I don't think the additional risk of adding some light DOM manipulation to implement spread modes is too great. (It's very snappy even on hundreds of pages, and limited to just that function.)
That said, these two commits implement the scheme I promised @prohtex above, with the scrolling modes and code fixes in the first commit, and the spread modes in the second:

The old ‘Grid Scrolling’ has been renamed to ‘Wrapped Scrolling’, which is hopefully less surprising to people discovering it. The spread options are toggle buttons (there are three states: neither selected, odd, or even), and supercede the old ‘Grid Scrolling with Cover Page’ mode. I didn't make any icons for them yet, but I can if this gets the green light.

Here are my responses to @Snuffleupagus's technical feedback:

Pre-rendering of the next/previous page, i.e. the one after/before the visible ones, depending on the scroll direction is currently not working correctly in the "Horizontal Scrolling" mode; see PDFRenderingQueue.getHighestPriority.
Currently the watchScroll method only tracks vertical scroll, since that's the only thing that mattered, however that's no longer correct here. While we might be able to hack around that here, I cannot help wonder if watchScroll shouldn't be re-factored to be more generic instead!?

Thanks for this catch! This was easily fixed by extending watchScroll in the obvious way and calling getHighestPriority with either scroll.right or scroll.down (depending on whether horizontal scrolling mode is currently enabled) instead of scroll.down always.

The single most serious problem with this PR, is that the getVisibleElements function will unfortunately not work in either the "Horizontal Scrolling" or "Grid Scrolling" modes for PDF documents with varying page sizes; e.g. https://github.com/mozilla/pdf.js/files/1513951/PageSizes_output.pdf

What can happen here, is that getVisibleElements will miss pages that are actually visible, leading to some pages failing to render; note this condition. I'm just not sure how we'll be able to address that, with only limited changes to getVisibleElements, such that we won't end up regressing performance for the regular (vertical) scrolling mode.
Given that it's imperative that getVisibleElements works correctly, in order for the viewer not to break in "interesting" ways, e.g. a complete re-write of getVisibleElements could pose some risk with only manual testing available (since it's difficult to cover all cases).

I see what you mean. I solved this in two steps: first, to handle the horizontal scrolling case, I made a second copy of getVisibleElements with all of the orientation words reversed (this prevents the possible performance regression and risk of error of attempting to parameterize one function to do both orientations). As this is a purely mechanical translation, I also wrote a unit test to verify that the two functions are identical modulo this difference. Second, to handle grid scrolling with unequally sized pages, I slightly modified the condition you highlighted. The net performance impact to the function in vertical scrolling mode should be one extra number comparison per visible page, plus half of an extra loop iteration—hopefully that's acceptable.

While this implementation obviously only makes sense for the PDFViewer, the PDFSinglePageViewer (or more likely BaseViewer) must still implement the same exact interface (with no-op methods) to prevent errors.

I moved the API added to PDFViewer down to BaseViewer.

As a side effect of expanding the number of visible pages, I noticed that the PDFPageViewBuffer's resize method was causing an odd behavior: it was destroying pages in the order they were rendered, without regard for whether those pages were still visible! This meant that there was often a flash of unnecessary re-rendering when switching between scrolling modes. I fixed this too, for all scrolling modes (it's possible for this to impact someone just doing regular vertical scrolling if their zoom is low enough, their screen is big enough and they resize their window).
Next round of reviews, please!

I should have trusted that the thing @Snuffleupagus said was the most serious problem would in fact be the most serious problem... whoops!
I've fixed all the issues I know about now. This PR is starting to get big, but the biggest chunks of new code are in ui_utils and consequently have test coverage. (The biggest uncovered chunk that isn't simple boilerplate is probably _regroupSpreads.)

Thank you so much @rhendric for your hard work on this. I have tested with a variety of PDFs and can see no issues. I showed it to a client and they absolutely loved it. "Spread lock" is intuitive and 2-up vertical looks awesome. I really, really hope this gets approved as it provides a vastly enhanced viewing experience. Here's a test with a very large PDF:
http://www.thing.gallery/test/scans/9788861306446-Made_In_Cassina.pdf

@rhendric truely amazing work. Some great comments and analysis and I like how you have taken it all onboard and produced a long awaited PR.
I wasn't able to test you latest code using http://54.67.70.0:8877/5e5412c2765c9bd/web/viewer.html (it still shows 'Grid View') but I could test with @prohtex link from above.
I have tested with some large PDFs and it is functioning well is all modes.
I do have one suggestion or request. In the Horizontal Scroll mode. Would it be possible to enable 'page scroll' using a key sequence (like CTRL + right arrow / CTRL _ left arrow). This would be similar to Vertical scroll using left and right arrows - it then scroll up and down by one page.
Currently using left or right arrow scrolling incrementally which is fine but often reading a book you wish to scroll by whole pages - so it could scroll by page width (plus margin) if CTRL _ left or right is used.
Once again thank you your work and I do hope that the powers to be here approve this PR
Darren

I'd like to add one note, and please, feel free to ignore this if this will in any way delay this PR. When I view a PDF, and then return to the same URL at a later date, pdfjs "remembers" my zoom level and which page I left off on. This is great. How is this accomplished? With a cookie? Surely, it would make sense for the viewer to also remember which viewing mode I was using, and to resume in that mode. I understand it may not be possible to provide a hook, preference, cookie, or other method for setting a default view mode, but it does seem intuitive that the viewer would remember the last view mode used.

@darrenahunter, the keys k/j and p/n also scroll back/forward by one page, and this continues to work regardless of scroll mode. I agree with you though that this could be more intuitive; I'm going to look into using up/down and page up/page down for this as well when there is no vertical scrollbar.
I did consider whether one click/key press should scroll to the next row instead of the next page, but I decided against attempting it. What about the case when you had spreads locked but you're zoomed in? In that case, going to the next page should really go to the next page, not the next row. But then what is the condition for switching between next page and next row? Is it when the next page is already fully visible? That's not consistent with how horizontal scrolling works, where the next page is frequently already fully visible but next page scrolls forward only one page instead of an entire screen. In the end, I decided simpler is probably better than smarter, at least for the initial iteration of this feature.
@prohtex, no promises, but I'll look into it! If this ends up being coupled to hash handling in any way, I'll probably skip trying to do anything about it, but otherwise I agree that it would be better if the viewer remembers.

I've been testing this PR with PDFs of all stripes on a number of platforms for the past week, keeping a close eye on performance and the JS console. So far I have not seen any issues not already present in this build: https://mozilla.github.io/pdf.js/web/viewer.html

Thanks for testing, @prohtex! I forgot to explicitly comment, but I did try to grant your wish for remembering scroll and spread state per document in my last set of commits; have you taken that for a spin yet?

Hi @rhendric
Is there a URL we can use to test all you changes.
Or do we need to clone the master and merge your PR ?
Very excited about your changes and happy to do as much testing as required
thanks

@rhendric I had some time earlier today but I couldn't figure out how to merge the new code. I will start from scratch and see if I can get it up and running to play around with the new additions. Thanks so much for that!
One thing I have noticed is that with spread lock on, it would also probably make sense to display the thumbnails side by side in the sidebar. Probably something to worry about later on, but it would make a lot of sense when browsing in those modes. Of course, grid scrolling is such a paradigm shift when it comes to viewing PDFs that perhaps few people would still use the thumbnail browser anyway.
I now prefer grid scrolling to my desktop PDF viewers of choice - Adobe Reader and Apple Preview. That's really saying something! This is such a massive improvement for reading books and viewing printed material. I can't thank you enough.

@darrenahunter, I'm a new contributor here, so I don't know all the infrastructure, but I think the situation is that someone on the Bot.io whitelist can deploy another test build, but until then you have to get the code yourself. (The fastest way to do that is probably to download and extract the archive from my fork.)
@prohtex, I don't know if you're aware, but if you resize the sidebar, those thumbnails already have their own wrapping behavior (which may actually make your observation more significant, since that wrapping doesn't respect spread locks). But I do agree that hopefully this is a next-iteration worry. And thank you for the kind words!

/botio-linux preview

Thanks so much for everyone contributing here ;)
@rhendric special thanks again to you for making this. Just seen one small issue: with the last version (  http://54.67.70.0:8877/5e5412c2765c9bd/web/viewer.html ) at least the setting "Grid Scrolling" was remembered when opening a new file (zoom setting seems to be handled via pdfjs.defaultZoomValue which is not available here). The new version http://54.67.70.0:8877/86639ee298277c0/web/viewer.html seems not to remember the setting "Wrapped Scrolling" and is just defaulting to "Vertical Scroll" when opening a new file.

@kekkc That is very odd. The inverse is true for me. The old version does not remember any of the new scroll mode settings, whereas the new version does.

What prohtex says is what I'd expect, @kekkc; I didn't write any code that would appear in the earlier build that was intended to remember scroll modes at all, for the same file refreshed or for a new file. Are you quite sure you didn't get things backwards somehow? If so, can you detail what you're doing to test so I can try to reproduce?

Hm, strange. What I did:

Install fresh FF 58.01 Portable
Open http://54.67.70.0:8877/5e5412c2765c9bd/web/viewer.html
Select "Grid Scrolling" & 100% zoom --> results in 2 pages on my screen
Open any other PDF in the same viewer via "Open file" button
Select 100% zoom again

Result: Again 2 pages are displayed and "Grid Scrolling" is still selected (however closing the complete tab and opening a new tab with http://54.67.70.0:8877/5e5412c2765c9bd/web/viewer.html is of course not remembering the "Grid scrolling" setting).
With the new version ( http://54.67.70.0:8877/86639ee298277c0/web/viewer.html ) the result is different and "Wrapped scrolling" is not remembered when opening a new file.

Ah, thank you! I see what's happening now. (I didn't even realize that ‘Open File’ button existed! It doesn't show up in the pdf.js embedded in Firefox, and I wasn't looking out for new buttons in the non-embedded version.)
So yes, this behavior did change—when I went to implement remembering the scroll modes per document, I tried to mirror the behavior of similar view settings like zoom[1], which, if the document hasn't been opened before, take their default values instead of preserving the value of the previous document. I think this is the correct thing to do, even in the long term, although it might be nice (in a later iteration) to add preferences for default scroll and spread modes the same way there's a preference for the default zoom value. Do you agree?
[1]: Are there others besides zoom? In my testing, things like current page, sidebar visibility, and page rotation don't seem to be correctly preserved per document, but there is code which suggests they should be, and I'm not sure what the deal is. So zoom is my point of reference for this.

@rhendric

I think this is the correct thing to do (that is, zoom and scroll/spread modes should do the same thing), even in the long term, although it might be nice (in a later iteration) to add preferences for default scroll and spread modes the same way there's a preference for the default zoom value. Do you agree?

Same thought here. Scrolling & spread absolutely deservere a proper setting in a later iteration (as all the other "pdfjs." settings in about config [pdfjs.defaultZoomValue, pdfjs.sidebarViewOnLoad, pdfjs.cursorToolOnLoad, pdfjs.enableHandToolOnLoad]). Most prominently "pdfjs.showPreviousViewOnLoad", which should remember Zoom & Sidebar by default, anyways (guess I'll open a seperate issue for this).
Most importantly, this PR gets another review to not delay this. @Snuffleupagus would you be able to support again?

@Rob--W:

I haven't reviewed the CSS changes, but when I glossed over I saw some value changes. For non-trivial changes (such as the changes of margin), can you explain why you made the change? We don't have tests for UI, so any changes should be justified to avoid unpleasant surprises.

All of those changes are basically because switching to display: flex changes how margins are interpreted. (I think margins are the only non-trivial changes to CSS values here.) The biggest change is that auto margins would induce unwanted space between flex items. So the role previously performed by auto margins is now served by align-items: center, and the margins have been set to values that ensure pixel-perfect reproduction of the pre-change viewer when in default modes (checked visually by switching between Firefox tabs).

Will this affect or interfere the behavior of printing?

I've tested printing PDFs in Firefox with all of these options, and as I'd expect, I didn't notice any changes to the behavior.

What happens in browsers that do not support the CSS here (intrinsic sizes via min-content)?

I haven't tested this in any such browsers, but I can use developer tools to disable those CSS declarations, and the effect is that horizontal scrolling breaks severely, and vertical scrolling can regress under some conditions (unequal page widths and viewport narrower than a page). I can prevent the latter regression by making the CSS more complicated (it would amount to walking back the display: flex and margin changes for vertical scrolling mode only), but I wasn't sure what our level of support for browsers that don't support intrinsic size CSS should be.

Which browsers have you tested?

Only Firefox and Chromium, both on Linux.

(Still not looked at the CSS yet, just replying to the comment).

All of those changes are basically because switching to display: flex changes how margins are interpreted. (I think margins are the only non-trivial changes to CSS values here.) The biggest change is that auto margins would induce unwanted space between flex items. So the role previously performed by auto margins is now served by align-items: center, and the margins have been set to values that ensure pixel-perfect reproduction of the pre-change viewer when in default modes (checked visually by switching between Firefox tabs).

Can you explicitly state the test plan that you used, so I can check it too and do additional checks if necessary? As stated, we don't have a good way to combat regressions, so I want to be thorough to not expose users to regressions when I publish an updated viewer with your new features.
What is the specific reason for using flexbox? Could CSS grids potentially be a better solution (perhaps it won't even require any changes to the HTML structure)?

I've tested printing PDFs in Firefox with all of these options, and as I'd expect, I didn't notice any changes to the behavior.

In Firefox, the printed pages are directly generated through JavaScript. In contrast, in Chrome (and other browsers), the printed pages are based on the rendered HTML (with @media print). To test you could print to PDF with Chromium and see if there is any difference.

I wasn't sure what our level of support for browsers that don't support intrinsic size CSS should be.

I'm not sure either. In PDF.js 1.x, we strived for a very wide range compatibility. In the current master, the upcoming 2.x release, the code base has been trimmed to drop support for older browsers. @yurydelendik / @Snuffleupagus What are the browsers that we want to support? https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support mentions some browsers, but without explicit version numbers for desktop Safari on macOS and mobile browsers.

Which browsers have you tested?

Only Firefox and Chromium, both on Linux.

In the future, please mention explicit version numbers. If at a later stage a new bug is discovered, knowing the version that you tested with makes it easier to determine whether the bug is a regression or a bug that always existed.
I tested with Chromium 64.0.3282.167 and Firefox 58.0.1 on Linux.

Can you explicitly state the test plan that you used, so I can check it too and do additional checks if necessary? As stated, we don't have a good way to combat regressions, so I want to be thorough to not expose users to regressions when I publish an updated viewer with your new features.

Test plan:

Open http://54.67.70.0:8877/86639ee298277c0/web/viewer.html in one tab
Open https://mozilla.github.io/pdf.js/web/viewer.html in a second tab
In both tabs, zoom to 90% (or to some number such that at least the top of the next page is visible on your monitor)
In both tabs, scroll to the very top of the document.
Toggle back and forth between tabs, looking for any differences in page positioning (there should be none).
In both tabs, scroll to the very bottom of the document. Repeat previous step.
Retry with different documents (I tested with the TraceMonkey default and https://github.com/mozilla/pdf.js/files/1513951/PageSizes_output.pdf, the latter at 10% zoom)

I did the above in Firefox 56.0.2 on Linux.

What is the specific reason for using flexbox? Could CSS grids potentially be a better solution (perhaps it won't even require any changes to the HTML structure)?

Flexbox was such a natural fit for the various things I wanted to achieve (except for spread locking). I'm not as familiar with grid layout, but it seems like that spec is all about having some external structure that you pour content into, whereas flexbox is more about having content items that already have opinions about their size and shape and arranging them nicely left-to-right or top-to-bottom. Some of the option combinations enabled by this PR can probably be done with grid layout—heck, some can be done with display: inline and neither flexbox nor grid layout. But I think only flexbox can do this:

I did consider grid layout to implement two-column page viewing, but when it was pointed out to me that the real need here is locking page spreads together, not literally viewing pages in two columns, I decided that flexbox plus the HTML hack for spreads was the cleanest way to go. (Maybe the HTML hack can go away someday, if the CSS Fragmentation Module gets widely implemented and break-after: avoid is ruled to apply to flex content.)

In Firefox, the printed pages are directly generated through JavaScript. In contrast, in Chrome (and other browsers), the printed pages are based on the rendered HTML (with @media print). To test you could print to PDF with Chromium and see if there is any difference.

Oh, that is really good to know! Printing the TraceMonkey PDF in Chromium (63.0.3239.132, Linux) with each of the options individually showed no change in behavior either.

Flexbox was such a natural fit for the various things I wanted to achieve (except for spread locking).

Flexbox is better.

I'm not a UX guy, so please take this with a grain of salt (and get opinions from other project members).
For UI consistency, wouldn't it be better to have an explicit menu item for OFF? Then you can also replace the spreadMode getter and setter with a setSpreadMode method, similarly implemented as setScrollMode.

I think the current approach makes more sense, unless all three options are grouped under a submenu.

I believe the the menu should auto close for those modes - as isn't it better to treat the user as if they know what they want and close the menu for them. Rather than coming from the perspective that the user could have made a mistake and lets leave the menu open so it is easy for them to change back - wouldn't this be catering for a small percentage at the expense of the many.

I agree with @darrenahunter here. Unless I'm mistaken, what @Rob--W is describing is a sticky menu that would stay visible until the user dismisses it. This approach would cater to a novice user who does not understand what a "grid" is, or what a "spread" is, in the context of viewing a PDF. That user could click the various options and see his/her changes reflected in real time. Perhaps that user would appreciate the menu staying open to ease toggling between the various view methods. But that user would soon come to understand how these display modes work, and then, like all non-novice users, would then be inconvenienced by having to click twice to close the menu after making a selection.
That is bad UX!

I'll resume review tomorrow or i the weekend. Thanks you for getting back so quickly, and thanks to the others who chimed in too.
I weakly prefer not auto-closing, but I can also understand the other pespectives here.
Let's await more feedback before making my requestedd change (i.e auto-close the menu for now).
@Snuffleupagus WDYT?

I'll resume review tomorrow or i the weekend. Thanks you for getting back so quickly, and thanks to the others who chimed in too.

Thank you @Rob--W for reviewing this PR. I think it is a game-changer for pdfjs.

I'm not sure either. In PDF.js 1.x, we strived for a very wide range compatibility. In the current master, the upcoming 2.x release, the code base has been trimmed to drop support for older browsers. @yurydelendik / @Snuffleupagus What are the browsers that we want to support? https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support mentions some browsers, but without explicit version numbers for desktop Safari on macOS and mobile browsers.

I'm implementing pdfjs for clients in the educational/nonprofit sector who work exclusively on iMacs, Macbooks, iPads and iPhones. Most are using Safari but some use Chrome on Mac. None use (or have even heard of) Firefox. There are quite a few bugs and UX quirks using pdfjs on Safari, especially on iOS devices. I'd love to dedicate more time to improving this experience, if it is a priority for this project.

Do we intentionally want to drop IE11 support?

+1 for dropping IE support ;)

There are quite a few bugs and UX quirks using pdfjs on Safari, especially on iOS devices.

Whoops! If I want to not add to that (and I don't), at the very least, it looks like I need to add a -webkit- prefix to at least one of my CSS tricks, based on caniuse.com.

@rhendric I've been trying to get a line of code added for a while (#6685) that will fix an issue where high-DPI assets are not served to Safari. I don't know if there is some bias or concern about webkit prefix.

@Rob--W do you have a perspective on creating a flag in the code that would allow a developer deploying pdfjs to set a default view and spread mode?

When I test your current demo, the page-fit mode does not actually fit in Chromium 64 and Firefox 58 on Linux: There is still a visible vertical scrollbar.

I'm also seeing this on Safari 11, but only in full-screen mode.
Another question: with spread-lock enabled, shouldn't clicking the "previous"/"next" buttons jump to the next 2-page spread, rather than the next single page?

I'm not a UX guy, so please take this with a grain of salt (and get opinions from other project members).
For UI consistency, wouldn't it be better to have an explicit menu item for OFF? Then you can also replace the spreadMode getter and setter with a setSpreadMode method, similarly implemented as setScrollMode.

After reviewing this, I'm inclined to agree there needs to be an "off" mode for spread lock. It is intuitive to me that I would click the active lock mode to turn lock off entirely, but may not be to the average user, especially given that the other menu items are stateless (selecting one that is already selected does not "turn it off").

@Rob--W do you have a perspective on creating a flag in the code that would allow a developer deploying pdfjs to set a default view and spread mode?

You can add preferences to control this. Then users and devs can both customize the default modes.
The most complete example of a user-visible, admin-configurable preference with multiple options is the cursorToolOnLoad option. At first it was a boolean option, later another option was added. You can either look at the source code, or review the differences at https://github.com/mozilla/pdf.js/commit/36c2791296adc4831a200fde5eed819781fc8174 and https://github.com/mozilla/pdf.js/commit/19549bb7d6191cc6e79cc806b0fbf9c0598db192.
The relevant preference-specific files in the above commits are:
extensions/chromium/options/options.html
extensions/chromium/options/options.js
extensions/chromium/preferences_schema.json
web/default_preferences.json
Here are examples of plumbing to propagate new preferences through various layers of the PDF.js components:
https://github.com/mozilla/pdf.js/commit/20d6286cceb936dfcb334ac1c963cf93c0da3637
https://github.com/mozilla/pdf.js/commit/ece44d36e8ff9bc7469e0952e4707858f0427e3f
(these examples are booleans, you would be using integers)

About the test plan from https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366106788: I don't expect anything spectacular with the default PDF, where all pages have the same size. I think that the more interesting (edge) cases appear in documents with varying page sizes.
When I have a (vertically) narrow window, many pages of the default PDF file fit in wrapped mode. However, arrow left/right will sometimes still not have any effect (i.e. the scroll position doesn't change, while I somewhat expected the next row to become visible). What is the expected behavior? Would a visual highlight (e.g. a focus ring around a page) make sense here?

@prohtex:

Another question: with spread-lock enabled, shouldn't clicking the "previous"/"next" buttons jump to the next 2-page spread, rather than the next single page?

Addressed in this comment.

After reviewing this, I'm inclined to agree there needs to be an "off" mode for spread lock. It is intuitive to me that I would click the active lock mode to turn lock off entirely, but may not be to the average user, especially given that the other menu items are stateless (selecting one that is already selected does not "turn it off").

Well, one clue that these are toggle buttons and not radio buttons is that they all start unselected the first time a new user opens that menu.

@Rob--W:

About the test plan from #9208 (comment): I don't expect anything spectacular with the default PDF, where all pages have the same size. I think that the more interesting (edge) cases appear in documents with varying page sizes.

That's just the test plan for checking that the new CSS margins and such match the old, visually. Honestly, while I did test with the PDF with varying page sizes (see last step in the test plan), discrepancies were all visible in the default PDF until I got it right. The relevant dimensions are the spaces between page edges and scroll container edges, and the vertical space between pages, none of which are really affected by varying page sizes.

When I have a (vertically) narrow window, many pages of the default PDF file fit in wrapped mode. However, arrow left/right will sometimes still not have any effect (i.e. the scroll position doesn't change, while I somewhat expected the next row to become visible). What is the expected behavior? Would a visual highlight (e.g. a focus ring around a page) make sense here?

I'll also direct you to this comment! The expected behavior is that you should see the page number advance one, at least (there was an earlier bug where sometimes even that wasn't happening, so if you don't see the page number advance, please flag), and then the page corresponding to that number should be fully visible, if possible. A focus ring is an interesting idea—I like it, but can that be deferred to a future iteration on this feature? I'd like to get the basics agreed on as quickly as possible.

The relevant dimensions are the spaces between page edges and scroll container edges, and the vertical space between pages, none of which are really affected by varying page sizes.

Good to know, thanks for this explicit statement. I wasn't sure whether your comment about not having visual differences applied to the test PDF only, or also the PDF with varying sizes.

A focus ring is an interesting idea—I like it, but can that be deferred to a future iteration on this feature? I'd like to get the basics agreed on as quickly as possible.

Yes, this is not a blocker. There are multiple nice-to-haves that don't need to block this PR. If you don't mind, could you create a list of known issues and potential improvements that you received from reviewers and testers/users? Then others can implement the ideas too if it isn't a priority for you.

I asked @yurydelendik for his opinion on the spread menu UI. We have no strong opinions on two or three menu items, but think that the menu labels are not immediately obvious. Currently it is:

"Lock Odd Spreads"
"Lock Even Spreads"
(when either of the two is clicked, the label doesn't change, but the meaning of the button becomes "Turn off page spreads".

I have no intention of blocking this for a prolonged time on bikeshedding about the menu labels, so quick suggestions are welcome.
Conversation from: https://mozilla.logbot.info/pdfjs/20180216#c14316805-c14316909
19:19:55 <@yury> robwu_nl: looking at the current UI, I think it's okay
19:20:18 <@yury> It might confuse first though, text must be selected carefully
19:21:16 <@yury> probably "Odd Two-Page Spread" (without "Lock")
19:21:49  * yury ihas no English major to make this call
19:22:15 < robwu_nl> To be explicit, you think that the menu [odd spread on] [even spread on] (with either button's meaning changing to [spread off]) is fine?
19:22:54 < robwu_nl> The text labels don't change when toggled, by the way.
19:22:55 <@yury> the meaning is to specify which spread book/PDF *already* has
19:23:12 <@yury> so it's more view configuration
19:23:57 <@yury> I wonder if there is already a property in PDF that says that
19:25:20 <@yury> robwu_nl: current way off button toggling is fine
19:25:39 <@yury> (just not sure about their labels)
19:28:07 <@yury> robwu_nl: if you feel that adding "No Two-Page Spread" button will help with UX, we need to add that
19:28:57 <@yury> having "No Two-Page Spread", "Odd Two-Page Spread", "Even Two-Page Spread" might be a good choice

Arguments in favor of using the word ‘Lock’:

The user who requested this feature, who works primarily with print materials, referred to this concept as ‘spread lock’.
It evokes what the configuration actually does: it locks two pages together. Synonyms like ‘join’ or ‘attach’ would also satisfy this point. Rephrasings like ‘Show Odd Spreads’, or simply ‘Odd Spreads’, don't really describe what that configuration does—‘Show’ would be actively misleading, since all pages are viewable regardless of the setting; and a mere ‘Odd Spreads’ doesn't suggest (to me, at least) what is going to change when the button is clicked. Note that the effect of toggling spreads is not always visible: in Horizontal Scrolling mode, the buttons have no visible effect, and in Wrapped Scrolling mode, if the zoom and window size are such that an even number of pages already fit side-by-side, toggling Odd Spreads won't change anything either (but Even Spreads will).

Arguments against:

Using a verb on a toggle button is perhaps the root cause of the discomfort with this label and the lack of an off button. A button that says ‘Sidebar’ can obviously be toggled on or off; a button that says ‘Show Sidebar’ is less obviously the thing to click when you want to hide the sidebar.

All that in mind: perhaps ‘Odd Spreads Together’? ‘Odd Spreads Joined’? I think ‘Odd Two-Page Spreads Together’ etc. is a little too verbose.

Addressed in this comment.

In that case, I think the focus ring is a good idea. Obviously not urgent.
Thanks everybody! Can’t wait to see this merged!

Adobe seems to call this "two-page view" or "Two-Up", according to https://superuser.com/questions/235887/how-to-make-the-first-page-of-a-pdf-display-by-itself-and-the-succeeding-pages-d and http://indesignsecrets.com/make-it-look-like-a-book-in-the-acrobat-pdf.php .
"Two-up" seems to not be a commonly used term (its definition is not related to books), so I ignored it and checked the word frequencies of phrases that contain "page spread". I am using the following query on the Ngram Viewer of Google Books' corpus (1900-2008), and the results, in order of frequency are:

"page spread" (most commonly found)
"two - page spread"
"double - page spread"
"one - page spread" (occurs rarely, possibly not significant?).
"single - page spread" (not found).

I also checked on DuckDuckGo for "two-page spread" and found multiple results about Microsoft Office, books and documents. Adobe uses the "two-page spread" term (and calls the feature itself "Two Page View").
"Page spread" seems to be the correct terminology, so let's stick to it, I guess?
"odd page spread" and "even page spread" are likely not common terms. To exclude results like 'this is an odd page spread", I looked for "uneven page spread" and found very few results in Google. So let's use something else.
I am thinking of less technical terms, so how about something like this:
.:  "Two-page spread (book with cover)"
::  "Two-page spread (book without cover)"
^^---- This is the icon; The dots there should be rectangles with numbers inside.

As you already know, page spreads are currently not functioning in horizontal scrolling mode. Would it make sense to disable the page spread menu items when horizontal scrolling mode is enabled?
Or do you intend to eventually support page spread in horizontal mode? The following could be a (contrived) use case:

Physically rotate my big screen so that it is in "portrait" mode.
Rotate all pages using PDF.js's menu option.
Enable horizontal scrolling mode (physically, the scroll direction is vertical again because of the rotated screen).
Enable page spreads.

@Rob--W "2-up", like "spreads," is a common print design term. The difference is, a 2-up can be anything--a poster, a gatefold, a diptych on the wall, etc. "Spreads" specifically refers to bound print matter. Adobe uses "2-up" to refer to displaying one spread (2 pages) at a time. @rhendric's scrolling modes go further in that you could see 2, 4, 6, or 20 spreads at a time, depending on zoom level. That's why I proposed the "spread lock" term, because what the viewer is doing is taking the single pages and locking them together to form spreads.
There is one thing I think this mode needs, and that is an adjustment to the margins between the pages to indicate where spreads lie. This would allow spread lock to function in horizontal mode as well. Below are some quick mockups.
On the topic of cover pages, there is another consideration. Books scanned to PDF also sometimes contain other material before or after the book content, such as publisher information, a scan of a spine or large gatefold dust jacket, etc. Some books have foldouts in the middle (called an "exception sheet") that are scanned as a single page, throwing off the grouping of spreads following that page. In short, there are a lot of reasons spreads would be locked on an even-page or odd-page basis--not just a cover. This is why I think the current terminology encompasses the broadest set of documents. Locking odd spreads implies no cover. Locking even spreads means the first page is excluded from the group, hence, cover. If a PDF begins with more than one exception sheet (like a scan of the inside dust jacket), those will simply be bunched together. Not every edge case can be accommodated. Maybe in the future more granular controls could be considered.
I understand pdfjs is used to display all sorts of documents, and scanned printed matter may not be the highest priority, but that is what I work with, and so what I can speak to.
Current view, 30% zoom, Wrapped, Lock Even:

Better:

Horizontal:

In some versions of Reader, Adobe takes this even further and pushes the two pages just one pixel apart, drawing a dotted line in-between. I think a subtle adjusting of the margins is probably better.

A footnote on how I came up with the term "lock:" in print, if there is a text area, image, etc, and it hits the gutter or margin, we say it is "broken." A newspaper "breaks" a blob of text into columns. Similarly, dynamically resizing a pdfjs window with wrapped scrolling enabled "breaks" the flow of pages into arbitrary bundles. In this case, a "lock" joining two pages together prevents the "break." I considered "join," "bind," "merge," etc, but I think "lock" is the clearest.

One final PPS on the topic of a "focus ring" proposed by @Rob--W. I remembered I have some clients who email around links to specific pages within PDF documents. For example:
http://54.67.70.0:8877/86639ee298277c0/web/viewer.html#page=4
If scrolling modes are enabled, and there is no focus ring (or other indicator for "active" page), and the sidebar is closed, it may not be clear which page is "current."

Have some new commits!
The test plans below were just executed in Chromium 63.
Issues addressed
Code changes per review

getVisibleElements rewritten for readability, no more duplicate getVisibleElementsHoriz

The asymptotic time of getVisibleElements should still be O(log(total pages)) + O(visible pages) except in very unusual cases not involving the default vertical mode. I haven't yet tried to benchmark this new version against either the previous or master versions.

scrollIntoView bug fix pulled out into its own commit, with commit message
buffer.resize and callers commented and refactored per code review
Speculative requestAnimationFrame removed

Better support for Page Fit with Horizontal Scrolling
Test Plan:

Open TraceMonkey PDF in Horizontal Scrolling mode, set zoom to Page Fit. Expect no vertical scroll bars.
Resize browser window to match the aspect ratio of one page (so decreasing either dimension of the window would cause the overall page to shrink), and then switch to Vertical Scrolling mode. Expect no horizontal scroll bars, and for the page to resize very slightly to account for the scroll bar moving from bottom to right.

(Note: this plan should not be expected to work with a PDF with unequal page sizes, due to Page Fit using the dimensions of the current page.)
Key handling rewritten for consistency across scrolling modes of special Page Fit keys.
Test Plan:

Open TraceMonkey PDF in Horizontal Scrolling mode, zoom out until there is no vertical scrollbar. Expect down arrow, j, and n keys to advance the page. Expect space, backspace, and enter to have no effect. Expect right arrow to scroll the viewport.
Set zoom to Page Fit. Expect down arrow, right arrow, j, n, space, and enter all to advance the page, and backspace to go back one page.
Choose Vertical Scrolling, and expect the same keys to have the same effects as in previous step.
Zoom out, and expect right arrow, j, and n keys to advance the page. Expect space to have its browser-default effect (scroll down by one screen). Expect enter and backspace to have no effect. Expect down arrow to scroll the viewport.
Choose Wrapped Scrolling, and expect the same keys to have the same effect as in previous step (check for advancing between pages by watching the page number box, since the viewport will not always scroll in this mode).
Open test/pdfs/freeculture.pdf in Horizontal Scrolling mode, scroll to a page in the middle of the document, and zoom to Page Fit. Expect down arrow, right arrow, j, n, space, and enter all to advance the page, and backspace to go back one page (all this despite both vertical and horizontal scroll bars being visible).
Set zoom to something else, and then switch to Presentation Mode. Expect the same keys to have the same effects as in previous step.
Leave Presentation Mode, switch to Vertical Scrolling, and repeat the previous step.

Outstanding issues and suggestions (blocking?)

Should we support browsers that don't understand (not even with a prefix) min-content CSS (IE/Edge, primarily)?
Should we add a No Spreads button?
Should we rename Lock Spreads, and to what?
Should we make some/all of the new buttons not auto-close the menu?

Outstanding issues and suggestions, non-blocking

Set scroll and spread modes via document URL
Set default scroll and spread modes as preferences
In Wrapped Scrolling and/or Lock Spreads modes, should previous/next page keys advance by page or by row/spread?
Should the Page Fit zoom preset fit an entire spread when a Lock Spreads is on?
Rewrite nearby classList.add/.remove calls to be consistent with added classList.toggle code
Add a focus ring for the current page
Squeeze margin between the two pages of a spread

Any chance we can have the 'bot' build a new viewer URL that we can all test from ?

/botio-linux preview

Viewer: http://54.67.70.0:8877/1994769d2882b14/web/viewer.html
Outstanding issues and suggestions, non-blocking

https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366489704
Just found 2 points that can be improved later on:

If "Wrapped Scrolling" is used and one clicks "Fullscreen" there is still only one page
Was mentioned somehow above, but I just realized that other tools like Foxit PDF Reader go back 2 pages if 2 pages are displayed. Means here a click on the page back/forwards would go as many pages back and forward that are currently displayed on the screen.
(Don't know if this is relevant, but FF 59 Devloper tells me pdf.js.xpi is not compatible [xpinstall.signatures.required=false])

Apart from these small things, I just want to highlighted that all other tools (Acrobat, Foxit, Sumatra, Okular...) implemented a 2 page view, but none is as advanced & consistent as this approach here. They all have some small quirks & differences (pages not vertically centered or not horizontally centered in some cases), that are not directly wrong, but also not consistent and as clean as this approach here ;)

More updates, testers needed (testing on IE11 and/or Edge is particularly desired, as I don't have either of those browsers to test with).
Issues addressed
CSS completely rewritten to (theoretically) support IE/Edge, possibly other less-cutting-edge browsers
All use of flexbox, intrinsic sizing, and browser vendor prefixes has been removed. It took less hackery than I was expecting, honestly, or I would have tried this earlier! It is a pretty delicate solution, though, so it could use some stress testing from others. (I've tested on Firefox 56.0.2 and Chromium 63, and they both seem to work just as well with the new CSS for me.)
Note: The observation I just made in my previous comment

[...] pages should be laid out next to each other to fill the space (which may include off-screen space if there is a horizontal scroll bar [...]

is no longer accurate. The new behavior of wrapped scrolling when there is a page wider than the viewport is to only wrap and center smaller pages to the viewport width, while still having a horizontal scrollbar so that the larger pages can be seen in full. In ASCII art:
+-------+    <- the rectangle is the viewport
|AAAAAAA|AAA
| BB CC |    <- DD would have had space here on this row in the previous version
| DD EE |
+-------+

... if you scroll right, you see
   +-------+
AAA|AAAAAAA|
 BB|CC     |
 DD|EE     |
   +-------+

I think I prefer this behavior, but it's an accident—I don't think I can get it to do the other thing without reintroducing flexbox and/or intrinsic sizing.
Added No Spreads button
Also dropped the word ‘lock’ from the spread button labels.
Code changes per review
This was mostly simplifying getVisibleElements and its helper backtracking function, and adding/rewriting comments.
Outstanding issues and suggestions, blocking
None???
Outstanding issues and suggestions, non-blocking

Set scroll and spread modes via document URL
Set default scroll and spread modes as preferences
In Wrapped Scrolling and/or Lock Spreads modes, should previous/next page keys advance by page or by row/spread?
Should the Page Fit zoom preset fit an entire spread when a Lock Spreads is on? Or should we add a Fit Spread preset that does this?
Rewrite nearby classList.add/.remove calls to be consistent with added classList.toggle code
Add a focus ring for the current page
Squeeze margin between the two pages of a spread
Should Presentation Mode show more than one page when spreads and/or wrapping is in use?

/botio-linux preview

The CSS change of adding white-space:nowrap may have side effects: Any text inside the pages that would previously wrap around will not wrap any more, and white-space that was pre. I consider this change to be acceptable and low-risk, because all divs in the text layer already have white-space:pre, and there are no other elements inside the page wrapper that could contain arbitrary text.

I'll add some CSS to .textLayer to reset white-space (and text-align) to initial, to further reduce the risk of unintended consequences.

The user-visible effect of this PR is that five buttons are added to the menu, which perform the actions as documented in this PR.

Six buttons, now that there's a ‘No Spreads’.

I'll add some CSS to .textLayer to reset white-space (and text-align) to initial, to further reduce the risk of unintended consequences.

That is not necessary. All divs in .textLayer are absolutely positioned.

Tests and comments added, _spreadMode renamed. @Rob--W, ready to sign off?

Apart from these small things, I just want to highlighted that all other tools (Acrobat, Foxit, Sumatra, Okular...) implemented a 2 page view, but none is as advanced & consistent as this approach here. They all have some small quirks & differences (pages not vertically centered or not horizontally centered in some cases), that are not directly wrong, but also not consistent and as clean as this approach here ;)

Agreed. I've been working with PDFs on a wide variety of systems since the spec emerged in the mid-90's. I've never seen a viewer employ a solution for viewing multiple pages that didn't feel "wrong" until now. @rhendric's scrolling modes are a huge achievement.

I have no more concerns about the implementation, but the tests need to be fine too, especially since other project members viewed the lack of UI tests as a maintainance risk. When these test coverage is good, I will approve the changes and request a review from another project member.

@Rob--W, are you referring to UI tests that currently exist, or ones that have yet to be created?

@rhendric I'd like to add a concern about cover pages to the list of non-blocking suggestions. When I posted about the margins between spreads, I forgot to mention this explicitly.
When viewing a PDF with a cover page, it feels natural that the cover page would be displayed "apart" from spreads. In Vertical Scrolling mode with Even Spreads selected, this is the case, as the first page is displayed on its own "row." Each subsequent "row" contains 1 grouping of 2 pages (a spread). In Wrapped Scrolling, as all pages are "wrapped," the cover page is not "apart" from the pages as it would be in Vertical Scrolling.
My suggestion is that Even Spreads would "break out" the cover page onto its own "row" regardless of scrolling mode. A mockup is below. Of course, this is enhanced by squeezing the spread margins as well.

I have no more concerns about the implementation, but the tests need to be fine too, especially since other project members viewed the lack of UI tests as a maintainance risk. When these test coverage is good, I will approve the changes and request a review from another project member.

@Rob--W, are you referring to UI tests that currently exist, or ones that have yet to be created?

I am referring to https://github.com/mozilla/pdf.js/pull/9208#issuecomment-359945496 (which was a reply to https://github.com/mozilla/pdf.js/pull/9208#pullrequestreview-90825961). We don't have a UI testing framework in place, but there are no drastic changest to the appearance in the current/default scroll modes, and a manual test plan is provided. And @rhendric has added provided unit tests for the (important) logic that is used to determine page visibility. I believe that these unit tests will get in a good shape within a few rounds of review.

BTW: Working with IE11 ;)

New commits, targeting CSS cleanup and improvements to tests.
All of your CSS feedback was great; everything you caught was indeed leftovers that had slipped through from the previous approach, and I now think that every line of CSS changed is necessary and accounted for. (I have been spot-checking the single page view example you pointed out, but now none of its CSS needs to be changed.)
For tests, I've rewritten the backtrack tests to be directly against the backtracking helper function, made constants for the pages of varying height in the significant row, commented all magic numbers, and added a horizontal scrolling test.

/botio-linux preview

Issues addressed:

Fixed discrepancy in distance between left edge of viewer and left edge of page when zoomed in, changing between vertical and horizontal scrolling modes
Discovered and fixed related discrepancy with that same distance changing between spreads and no spreads in vertical scrolling mode
Rewrote margin directives in CSS to only alter horizontal margins in lines touched by this PR
Added comment to tests on how to interpret inputs to makePages, and why BORDER_WIDTH gets involved inside makePages

I'll resume review tomorrow or in the weekend. One thing that I forgot to mention before is that when I switch between vertical and wrapped mode, that the scroll position changes (seemingly to align the top of the page), even if there is no need to change the scrolling offset (e.g. when the number of columns in wrapped mode matches the number of columns in vertical mode, i.e. 1 without spreads and 2 with locked spreads).

/botio-linux prevew

One thing that I forgot to mention before is that when I switch between vertical and wrapped mode, that the scroll position changes (seemingly to align the top of the page), even if there is no need to change the scrolling offset (e.g. when the number of columns in wrapped mode matches the number of columns in vertical mode, i.e. 1 without spreads and 2 with locked spreads).

Non-blocking issue, I hope? I agree this could probably be improved, but there would be a few ways to do it and maybe other scroll-automatically behaviors should be considered as well. (For example, maybe the rule should be don't scroll if the target page is already fully visible—but then does this rule apply when you click on a thumbnail in the sidebar as well? If we add a focus ring later, perhaps that changes the calculus on both questions?)

/botio-linux preview

@Snuffleupagus Can you take a last look?

Unfortunately I really won't have time do that, sorry!
However, I really want to release version 2.0 first, see issue #9521 (which is close to finished), before we even consider landing anything of this size to reduce risk in the initial release from the 2.0 branch.

@Snuffleupagus I have already performed an in-depth review, and thought that you wanted to take another look too following your previous review at: https://github.com/mozilla/pdf.js/pull/9208#pullrequestreview-90825961
Do you intend to eventually review this PR (perhaps a not-so-time-consuming shallow code review and/or only a review of functionality/UX)? If not, I will look for another team member right away. Otherwise we can await your review first.
(in either case, this PR should wait until 2.0 is released, which is soon, hopefully!)

PR updated to merge cleanly atop #9493.

Hopefully 2.0 is released quickly so this PR gets available in FF stable at some time.
How can I use the http://54.67.70.0:8877/e094be8db6da9bb/extensions/firefox/pdf.js.xpi ?
I installed it successfully in FF Developer Edition ( xpinstall.signatures.required: false, extensions.legacy.enabled: false ), but FF is still using the old version and not the extension. Unfortunately nothing is mentioned in the FAQ or anywhere else :( I want to use it as my main, permanent PDF reader.

I'd like to echo @kekkc's comments. I really hope this can be merged in a reasonable timeframe. I'd like to deploy this on a number of production sites without forking from base. I'm sure issues will crop up, but it seems like a tremendous amount of work has gone into the review process already. From what I can see, @rhendric is actively responding to all requests and has dedicated quite a bit of time and energy to this PR. Wouldn't it make sense to deploy this while the code is fresh in our minds and @rhendric is available to respond to issues from a wide user base?
Is there a timeframe for the 2.0 release? Is there a compelling reason why this PR couldn't be merged before the 2.0 release? I imagine there's a number of other changes that 2.0 will introduce that will also need testing. After watching several alternate approaches to "2-up view" wither on the vine over the course of seven years, I'm just really hoping this PR is supported and merged with base.
Thanks again to everyone who dedicated so much time and energy to this. It is truly a revolutionary, fully-featured solution and I can't wait to see it in wide use.

@kekkc The xpi file doesn't work because Firefox 57 and later only support WebExtensions.
@prohtex The next major release is near (with many major changes). To minimize (stability) risks (and not delay the release any further), we don't want to merge significant feature changes at the last minute. The release itself is tracked in #9521 (and the list of remaining tasks is at https://github.com/mozilla/pdf.js/projects/5 - basically documentation only). There is no fixed date yet, but it can't take that much longer.

/botio-linux preview

Months later, there seems to be no clear timeframe or means to deploy this code in either an official release or beta channel. Please help us.

Per IRC conversation, I've added a commit that lets the default scrolling and spread modes be defined via preferences. @Rob--W, review please?

This looks great. Thanks for your work, responsiveness and patience throughout this whole PR!

@rhendric @prohtex Can you reply to https://github.com/mozilla/pdf.js/pull/9208#issuecomment-390971058 ?
@flodolo There has been some previous discussion about the spreads terminology from https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366322219 onwards.
Here are some references to similar features in other software (MS Office, Adobe Reader): https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366384326
Can you enumerate the phrases that you consider difficult to translate?

Thanks for the link. I was indeed able to find some references to "page spreads" in Microsoft before, even if they usually refer to either "one-page spread" or "two-pages spread".

Can you enumerate the phrases that you consider difficult to translate?

Right now, the biggest problem is "wrapped scrolling".

Thanks for the link. I was indeed able to find some references to "page spreads" in Microsoft before, even if they usually refer to either "one-page spread" or "two-pages spread".

Can you enumerate the phrases that you consider difficult to translate?

Right now, the biggest problem is "wrapped scrolling".

I'm confident that after the rigorous debate we had on the matter, the current terminology is both accurate, specific, and understandable. The problem of translating a label into another language is just that -- a problem for someone who understands both English, and that other language, well. Furthermore, simply Googling the names of these terms won't necessarily yield anything useful about their fitness for this given featureset.
If someone wants to come forward with a widely understood, common practice term for "grouping of two opposing pages meant to be experienced as a single layout unit" I'm eager to hear it. Until then, I still feel "spread" is the correct term. As for "wrapped scrolling," as @rhendric has mentioned, "wrapping" is a widely understood print and web term that seems, to me, to be well-suited here. It very simply describes what is happening -- the pages are being wrapped. Yes, "wrapping" could be broken out into a toggle, but it would only be useful in horizontal mode. An alternative label name then might be the clunky "Horizontal scrolling with wrapping," although I can't see what additional clarity that would provide. At this point, unless real bugs or issues are discovered with this PR, maybe the best course of action would be to table the endless debate about terminology in favor of letting someone actually use it.
I'd encourage anyone who wonders how these particular features and labels were settled upon to scroll up and avail themselves of the extensive discussion on the matter.

I think it would be "Vertical scrolling with wrapping", right?

Please excuse me, yes, corrected above.

When I put "two-page spread" into Google translate for Spanish, for example, I get this:
propagación de dos páginas
I do not know Spanish but this appears to be something like "propagates across two pages."
When I enter "spread her legs," I get this:
extiende sus piernas
This indicates to me there might be a useful distinction here.

This entry in Microsoft Terminology may also be helpful:

English
Translation
Definition

wrap panel
panel de ajuste
A  layout container that arranges its child elements in a single vertical  or horizontal row, creating a new row if needed to display the overflow  from the previous row.

@flodolo
I came up with a different naming scheme by taking inspiration of Libre Office, Adobe Reader and Edge.
@rhendric
two page spread is literally "double page layout/design"
Afaik there doesn't exist an equivalent for page spread in German (at least not a short one)
about wrapping: the problem is not that we don't know what wrapping means. It's just that in German "wrap-around" doesn't exist at all.
And we can't make something up (if we want people to understand it).
else there would only be a full transcription of what it actually means in the hover title
@collinanderson "Vertical scrolling with wrapping" is too wide for that menu, I think
@rhendric
We could translate wrapping with the German version of "grid" or "table". That would work.
But there are other terms (see the linked issue) known to users of widely used software, that describe the same even better.
What do you think about my suggestions in the other issue?

@Djfe I'm not sure I understand why "Double Page Layout/Design" is any more succinct, specific, or widely understood than "Spread." Your proposal is to change
No Spreads
Odd Spreads
Even Spreads
to
No Double Page Layout/Design
Odd Double Page Layout(s)/Design(s)
Even Double Page Layout(s)/Design(s)
?
My feeling is still that it is not unusual for text to be longer in translation than it is in English. Often a product will be labelled "Door Handle" in English and something like "Lo que una persona se convertiría con el propósito de abrir una puerta" in Spanish.
Not to be ethno-centric, but I think the challenge is to come up with a set of terms for each localization that is true to that language, even if it must be a bit longer.
Translating "wrapping" to "grid" makes a lot of sense to me, as that is what this feature was called to begin with (grid scrolling).

My last comment, wasn't a reply to you (ups)
Just an amendment to my last post
This will be my reply:
"Double Page Layout/Design" is the known term in German
"doppelseitiges Layout" (double-paged layout)
page spread doesn't have an equivalent (in the public) here in Germany
probably there is a specific German term from the publishing domain, that I can't find with Google right now.

Your proposal is to change
No my actual proposal is in #9813

The rest were just ideas floating in my head, from before started looking at other software to get some inspiration.
And the slash in there was just: you could use either. (not both at the same time)

Translating "wrapping" to "grid" makes a lot of sense to me, as that is what this feature was called to begin with (grid scrolling).

yeah I just discovered that term, while trying to read all posts from this discussion (I'm still not finished, yet) hehe
for the second post of yours:

"Seitenlayout" is something that is immediately clear.

exactly, but noone ever heard of a page layout that could be even or odd
Everyone would ask in their minds what about it is supposed to be even or odd?
If you see the icons it kind of becomes clear.
even -> start ordering at page 2
odd -> start ordering at page 1

How about "One Page Layout" and "Two Page Layout," or "Single Pages" and "Double Pages"?

you should take a look at #9813 :)

In german when you talk about even or odd, people tend to think:
Ah, ok the number of pages is even or odd
Another way to look at it:
The problem I see is, that a spread always consists out of one even and one odd page, so calling the page layout/page spread just even/odd is confusing.
It doesn't tell you right away, that it refers to the first page of the duo.

Desktop Publishing Software seems to refer to page spread in the following:
"We support spread management."
"Wir unterstützen Spread-Management." (no translation, just the english name germanized, because it doesn't exist here)

@Djfe, I took a look at #9813. It seems like you've put a lot of thought into this. If this were something that was just introduced, I'd say it is worthy of debate. But I just took 20 minutes and scrolled up on this thread.
https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366302852
https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366328887
https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366384326
https://github.com/mozilla/pdf.js/pull/9208#issuecomment-367543936
As you can see there were quite a few other suggestions for how these menus might be organized. There was discussion of a submenu. At one point the spread lock was a 2-way toggle with no explicit "off" state. In short, quite a few options were considered. I'm pretty confident what we arrived at is both a good compromise, and quite clear from a UX standpoint.

Desktop Publishing Software seems to refer to page spread in the following way: "We support spread management."

https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366412385

@Djfe, @prohtex, I propose moving this conversation to #9813, despite there already being a lot of prior discussion on the topic of UX and names here. This PR's conversation thread is already enormous, and moving to #9813 would make it clearer when (and if) there is a resolution satisfactory to all.

@rhendric, thanks for your comments on #9813. When I have a chance I will review more fully and chime in. I just wanted to say that I think there is a little more than can be done to make "spread view" (or whatever you want to call it) useable, apart from renaming the various options. The main improvement from my perspective would be adjusting the page margins. Currently, with "even spreads" enabled, a zoom level of 50% or so looks great, with the cover page on its own row and subsequent pages side by side. Zoom out further however and things get strange. This was my comment here: https://github.com/mozilla/pdf.js/pull/9208#issuecomment-366412385. In any case, thanks as always for your dedication and I'm thrilled things are moving forward at long last.

authors: Djfe Rob--W Snuffleupagus collinanderson darrenahunter fgilio flodolo jlac1024 kekkc prohtex rhendric timvandermeij


text:
Bug 1148805 - Do escaping of AVM1 property names before iteratively searching through scopes instead of doing it for each scope over and over again

While it fully works and gets some performance improvements, this is WIP-py in that it could be extended to other operations. f? @yurydelendik on the approach.

/botio test

It might actually make sense to land this as-is: _escapeProperty completely vanishes from all profiles with it, whereas it takes close to 10% of execution time (excluding rendering) without it.

Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

src/avm1/interpreter.ts, line 885 [r1] (raw file):
variableName can contain a path to the child, e.g.  '../a', './a/b:prop', 'a.b.proto' -- escaping must be happening to all its parts.

src/avm1/runtime.ts, line 598 [r1] (raw file):
we don't need to name.indexOf(ESCAPED_PROPERTY_PREFIX) !== 0 to avoid name collision when user will specify e.g. __avm1__proto__. (Which also mean we shall never call alToName twice on the same name -- did you have a case when we potentially do that?)

Comments from the review on Reviewable.io

@yurydelendik, I removed the escaping and optimized a few other things. Makes the particle effects test case about 25% faster for me.

/botio reftest

/botio test

Review status: 0 of 14 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

src/avm1/interpreter.ts, line 954 [r1] (raw file):
This comment was not addressed. We can remove it -- the avm1ResolveSimpleVariable logic will call alHasProperty which will normalize the name.

src/avm1/runtime.ts, line 66 [r3] (raw file):
We can remove ESCAPED_PROPERTY_PREFIX

Comments from the review on Reviewable.io

/botio test

/botio test

@yurydelendik, I fixed the SetVariable issue we discussed yesterday in a follow-up patch in #2363.

authors: tschneidereit yurydelendik

repo: mozilla/shumway

license_source: bigquery


text:
bot should try harder in getting its name back

Our assumption here was that whenever it does not get its name, there's likely a stale connection from a previous instance that is lingering in the same channel. Seeing someone quit with the nickname "standups" makes the bot go back.
This does not appear to cover all cases. I just came online and noticed it had the wrong name. I started a 2nd irc session named "standups" and quit again, so our bot would see this.
It would be better to have a regular check in the bot to see if it can get its nickname back.
The IRC server we use supports the [watch module(https://wiki.inspircd.org/Modules/watch), where you add nicknames to your watch list and get notified when they appear and disappear. This could be used whenever the main nickname is not available.
Outline of patch:
On register: check own name. If name != config.name, send to server: watch +<name>
On watch-replies, take nick if it becomes available, then clear watch list (watch C)
:belew.mozilla.org 605 foo foobert2 * * 0 :is offline
:belew.mozilla.org 600 foo foobert2 freddy moz-mdovjj.dynamic.kabel-deutschland.de 1474268187 :arrived online

I'll try a pull request unless someone disagrees heavily.

This sounds good to me!

authors: mozfreddyb willkg

repo: mozilla/standup-irc


text:
Debian buid fail

Hi
When i try i Debian building i receive:
root@debian:/home/fabrizio/Scrivania# npm install tofino
npm WARN engine tofino@0.1.0: wanted: {"node":">=5.8.0"} (current: {"node":"4.3.1","npm":"1.4.21"})
npm WARN optional dep failed, continuing fsevents@1.0.11
|

tofino@0.1.0 postinstall /home/fabrizio/Scrivania/node_modules/tofino
electron-rebuild

sh: 1: electron-rebuild: not found
npm WARN This failure might be due to the use of legacy binary "node"
npm WARN For further explanations, please read
/usr/share/doc/nodejs/README.Debian
npm ERR! tofino@0.1.0 postinstall: electron-rebuild
npm ERR! Exit status 127
npm ERR!
npm ERR! Failed at the tofino@0.1.0 postinstall script.
npm ERR! This is most likely a problem with the tofino package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! electron-rebuild
npm ERR! You can get their info via:
npm ERR! npm owner ls tofino
npm ERR! There is likely additional logging output above.
npm ERR! System Linux 4.4.0-1-amd64
npm ERR! command "/usr/bin/nodejs" "/usr/bin/npm" "install" "tofino"
npm ERR! cwd /home/fabrizio/Scrivania
npm ERR! node -v v4.3.1
npm ERR! npm -v 1.4.21
npm ERR! code ELIFECYCLE
npm ERR!
npm ERR! Additional logging details can be found in:
npm ERR! /home/fabrizio/Scrivania/npm-debug.log
npm ERR! not ok code 0
root@debian:/home/fabrizio/Scrivania#

This might be because of the version of Node.js that you're using. It looks like you're on 4.3.1 but the package.json specifies >=5.8.0. Also, you'll want to update your version of npm.

Upgraded node and npm  to 5.8.0 but now i have:

root@debian:/home/fabrizio/Scrivania# npm install tofino

tofino@0.1.0 postinstall /home/fabrizio/Scrivania/node_modules/tofino
electron-rebuild

sh: 1: electron-rebuild: not found
npm WARN optional Skipping failed optional dependency /chokidar/fsevents:
npm WARN notsup Not compatible with your operating system or architecture:
fsevents@1.0.11
npm WARN enoent ENOENT: no such file or directory, open
'/home/fabrizio/Scrivania/package.json'
npm WARN Scrivania No description
npm WARN Scrivania No repository field.
npm WARN Scrivania No README data
npm WARN Scrivania No license field.
npm WARN You are using a pre-release version of node and things may not
work as expected
npm ERR! Linux 4.4.0-1-amd64
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "install" "tofino"
npm ERR! node v5.9.2-pre
npm ERR! npm  v3.8.6
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn
npm ERR! tofino@0.1.0 postinstall: electron-rebuild
npm ERR! spawn ENOENT
npm ERR!
npm ERR! Failed at the tofino@0.1.0 postinstall script 'electron-rebuild'.
npm ERR! Make sure you have the latest version of node.js and npm
installed.
npm ERR! If you do, this is most likely a problem with the tofino package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     electron-rebuild
npm ERR! You can get information on how to open an issue for this project
with:
npm ERR!     npm bugs tofino
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls tofino
npm ERR! There is likely additional logging output above.
npm ERR! Please include the following file with any support request:
npm ERR!     /home/fabrizio/Scrivania/npm-debug.log

With webpack:

root@debian:/home/fabrizio/Scrivania# npm install webpack -g
/usr/local/bin/webpack ->
/usr/local/lib/node_modules/webpack/bin/webpack.js
/usr/local/lib
└── webpack@1.12.15
npm WARN optional Skipping failed optional dependency
/webpack/chokidar/fsevents:
npm WARN notsup Not compatible with your operating system or architecture:
fsevents@1.0.11
npm WARN You are using a pre-release version of node and things may not
work as expected
root@debian:/home/fabrizio/Scrivania#

2016-04-12 20:22 GMT+02:00 Lin Clark notifications@github.com:

This might be because of the version of Node.js that you're using. It
looks like you're on 4.3.1 but the package.json specifies >=5.8.0. Also,
you'll want to update your version of npm.
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/mozilla/tofino/issues/91#issuecomment-209041016

Now i'm trying with 5.8 stable instead of 5.9...
2016-04-12 22:19 GMT+02:00 fabrixx fabrixx2@gmail.com:

Upgraded node and npm  to 5.8.0 but now i have:

root@debian:/home/fabrizio/Scrivania# npm install tofino

tofino@0.1.0 postinstall /home/fabrizio/Scrivania/node_modules/tofino
electron-rebuild

sh: 1: electron-rebuild: not found
npm WARN optional Skipping failed optional dependency /chokidar/fsevents:
npm WARN notsup Not compatible with your operating system or
architecture: fsevents@1.0.11
npm WARN enoent ENOENT: no such file or directory, open
'/home/fabrizio/Scrivania/package.json'
npm WARN Scrivania No description
npm WARN Scrivania No repository field.
npm WARN Scrivania No README data
npm WARN Scrivania No license field.
npm WARN You are using a pre-release version of node and things may not
work as expected
npm ERR! Linux 4.4.0-1-amd64
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "install"
"tofino"
npm ERR! node v5.9.2-pre
npm ERR! npm  v3.8.6
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn
npm ERR! tofino@0.1.0 postinstall: electron-rebuild
npm ERR! spawn ENOENT
npm ERR!
npm ERR! Failed at the tofino@0.1.0 postinstall script
'electron-rebuild'.
npm ERR! Make sure you have the latest version of node.js and npm
installed.
npm ERR! If you do, this is most likely a problem with the tofino package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     electron-rebuild
npm ERR! You can get information on how to open an issue for this project
with:
npm ERR!     npm bugs tofino
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls tofino
npm ERR! There is likely additional logging output above.
npm ERR! Please include the following file with any support request:
npm ERR!     /home/fabrizio/Scrivania/npm-debug.log

With webpack:

root@debian:/home/fabrizio/Scrivania# npm install webpack -g
/usr/local/bin/webpack ->
/usr/local/lib/node_modules/webpack/bin/webpack.js
/usr/local/lib
└── webpack@1.12.15
npm WARN optional Skipping failed optional dependency
/webpack/chokidar/fsevents:
npm WARN notsup Not compatible with your operating system or
architecture: fsevents@1.0.11
npm WARN You are using a pre-release version of node and things may not
work as expected
root@debian:/home/fabrizio/Scrivania#

2016-04-12 20:22 GMT+02:00 Lin Clark notifications@github.com:

This might be because of the version of Node.js that you're using. It
looks like you're on 4.3.1 but the package.json specifies >=5.8.0. Also,
you'll want to update your version of npm.
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/mozilla/tofino/issues/91#issuecomment-209041016

This my situation after fixing node & npm:

root@debian:/home/fabrizio/Scrivania# npm install tofino

tofino@0.1.0 postinstall /home/fabrizio/Scrivania/node_modules/tofino
electron-rebuild

sh: 1: electron-rebuild: not found
/home/fabrizio/Scrivania
└── webpack@2.1.0-beta.5
npm WARN optional Skipping failed optional dependency /chokidar/fsevents:
npm WARN notsup Not compatible with your operating system or architecture: fsevents@1.0.11
npm WARN enoent ENOENT: no such file or directory, open '/home/fabrizio/Scrivania/package.json'
npm WARN Scrivania No description
npm WARN Scrivania No repository field.
npm WARN Scrivania No README data
npm WARN Scrivania No license field.
npm ERR! Linux 4.4.0-1-amd64
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "install" "tofino"
npm ERR! node v5.8.0
npm ERR! npm  v3.7.3
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn
npm ERR! tofino@0.1.0 postinstall: electron-rebuild
npm ERR! spawn ENOENT
npm ERR!
npm ERR! Failed at the tofino@0.1.0 postinstall script 'electron-rebuild'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the tofino package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     electron-rebuild
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs tofino
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls tofino
npm ERR! There is likely additional logging output above.
npm ERR! Please include the following file with any support request:
npm ERR!     /home/fabrizio/Scrivania/npm-debug.log
root@debian:/home/fabrizio/Scrivania#

This is my npm-debug.log:
https://goo.gl/UrcTnf

The problem is that the Node.js binary in Debian is /usr/bin/nodejs, not /usr/bin/node. Authors should fix build scripts. Read /usr/share/doc/nodejs/README.Debian for more details.

Il 13 apr 2016 6:13 AM, "ddomgn" notifications@github.com ha scritto:

The problem is that the Node.js binary in Debian is /usr/bin/nodejs, not
/usr/bin/node. Authors should fix build scripts. Read
/usr/share/doc/nodejs/README.Debian for more details.
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/mozilla/tofino/issues/91#issuecomment-209218491

I not use Debian packeged version, ive builded all from sources.
Ive not usr/bin/nodejs file but
/usr/local/lib/node_modules
folders ---------->npm/tofino(symlink tolibrary)/webpack
/usr/local/bin:
---------->  webpack npm   symlynk to module folder
------------> node binary file
Than you for your patience..

Have you tried aliasing node as @ddomgn mentioned, wherever your node binary is?
I don't think this is an issue with this project

I don't believe this is an issue with this project. Closing for now, reopen if I'm wrong.

authors: ddomgn fabrixx jsantell linclark

repo: mozilla/tofino


text:
Create mapping conditionally (bug 1132264)

Fixes the regression I caused :(
More evidence basic tests would be good to cover this sort of thing. I'm beginning to think something basic is worth doing unless refactoring the devhub is coming soon?

r+
I agree we should try doing basic tests now. Even if it means trashing them later when the refactor finally takes place (I have been hearing about this refactor since I joined Mozilla...)

authors: diox muffinresearch

repo: mozilla/zamboni


text:
Remove mkt.webapps.views.BaseFilter (bug 1144663)

https://bugzilla.mozilla.org/show_bug.cgi?id=1144663
BaseFilter class is something we inherited from AMO. It was created 5 years ago, does a lot of stuff we don't really need anymore, and in a weird way. It should be much cleaner and reliable to simply use queryset filters directly.

Was just looking at this wondering if we could get rid of it. and you added more tests!
r+

authors: diox washort


text:
sample certs that violate BRs

We should make sample certs that violate BRs available through the UI at some point, through searching by root CA.

New table:
issuer, for each BR violation: {violation}ExampleCertPEM
(just keep the most recent one we've seen for each)

authors: monicachew mozkeeler

repo: mozkeeler/sunlight

license: mit


text:
download duplicated torrent can not worked

i think this is a technical bug
i create Client Object from a Torrent file and started and then make another Client from same torrent file and started
first initial Client can downloaded Torrent file , second Client put a lock in this torrent and torrent file can not complete , then if i stopped both Client  and started download again by one Client Object can not worked and i must be deleted torrent files for new Client .

I'm not sure I fully understand your problem, but two Client instances can't be downloading onto the same local files. They'd need to be on different directories.

authors: azimot mpetazzoni

repo: mpetazzoni/ttorrent


text:
WARNING: Too many active WebGL contexts. Oldest context will be lost. pannellum.js:12

Hi,
I use Pannellum to show my panoramic images with the map. As a general logic, when I click on the point on the map, the panoramic viewer works. My code worked, but after clicking on the 16th point, I get a webgl error (WARNING: Too many active WebGL contexts. Oldest context will be lost. pannellum.js:12) and my map screen disappears.

 figure1: Before The Error

 figure2: After The Error
Every time I click on the point on the map, it creates a new viewer. What can I do about it? The relevant parts of the code are below.
async function getAttachments(featureLayer, objectIds) {
      const attachmentRes = await featureLayer.queryAttachments({
        objectIds
      });
      attachments = attachmentRes[objectIds];
    }

    function handleAttachmentNodes() {
      clearAttachmentNodes();
      generateAttachmentNodes();
    }

    function clearAttachmentNodes() {
      const sidePanel = document.getElementById("sidePanel");
      while (sidePanel.firstChild) {
        sidePanel.removeChild(sidePanel.lastChild);
      }
    }

    function generateAttachmentNodes() {
      attachments.forEach((attachment) => {
        const attachmentNode = createAttachmentNode(attachment);
        panorama = attachment.url
 });
      const pv = PanViewer(panorama)
      console.log(1)
}

    function PanViewer(panorama) {
      const viewer = pannellum.viewer('sidePanel', {
        author: "Samed Adalı",
        autoLoad: true,
        type: "equirectangular",
        panorama: panorama,
      })
}

    function createAttachmentNode(attachment) {
      const attachmentNode = document.createElement("img");
      attachmentNode.classList.add("attachment-item");
      attachmentNode.src = attachment.url;
      return attachmentNode;
    }

Thanks in advance for your help.

What can I do about it?

You can create a single viewer the first time and then reuse it subsequently. You can do this by adding each subsequent image as a new scene with addScene and immediately switch to it with loadScene (and optionally then use removeScene to remove the previous scene configuration).

Can you share with me a sample code of addScene and loadScene? I haven't seen any examples. I read from the document, but I couldn’t succeed. I’m a kind of new to this stuff.

Hi,
For anyone else reading this in the future, I defined an empty viewer and destroyed the previous viewer in the if loop.
let viewer = '';
function PanViewer(panorama, att_id,scene) {
    if (viewer) {
      viewer.destroy();
    }
 const viewer = pannellum.viewer('sidePanel', {
        author: "Samed Adalı",
        autoLoad: true,
        type: "equirectangular",
        panorama: panorama,
      })

I wouldn't recommend doing that, since you can still end up with WebGL context issues that way. It's better to use something along the lines of (I'm assuming panorama is a URL, i.e., a string):
let viewer;
function PanViewer(panorama) {
    if (!viewer) {
        // If viewer doesn't exist, create it in tour mode with the first panorama
        const config = {
            'default': {
                'autoLoad': true,
                'type': 'equirectangular',
                'firstScene': panorama,
            },
            'scenes': {}
        }
        config.scenes[panorama] = {panorama: panorama};
        viewer = pannellum.viewer('sidePanel', config);
    } else {
        // If the viewer already exists, add the new panorama as a new scene,
        // switch to it, and remove the old scene configuration
        const oldScene = viewer.getScene();
        viewer.addScene(panorama, {
            panorama: panorama
        });
        viewer.loadScene(panorama);
        viewer.removeScene(oldScene);
    }
}

Hi Matthew,
Yes panorama is a url. I tried your code but it doesn't work. The else part in the code does not bring the new panorama.

figure1: clicked first point

figure2: clicked second point

I use v2.5.6 and the latest development version and there are no error messages in the developer console. I try chrome and opera.

Yup - I couldn't get this working either . Loads up fine, but won't load a new scene (or the old screen either) . Not sure why.  Am integrating it with a Vue3 project.
 if(!viewer)
      {
        const config = {
            'default': {
                'autoLoad': true,
                'type': 'equirectangular',
                'firstScene': 'test',
            },
            'scenes': {}
        }

        config.scenes['test'] =
        {
          panorama: props.image,
          haov: props.haov,
          vaov: props.vaov,
          autoLoad: true,
          showControls: false,
          hfov: 100,
          yaw: props.yaw,

        };

        viewer = window.pannellum.viewer('pano', config); // this loads up just fine :)
        viewer.setYawBounds(props.bounds);

      }else{

        const oldScene = viewer.getScene(); // this sees the correct scene Id
        viewer.addScene('newScene', {
            panorama: props.image,
            haov: props.haov,
            vaov: props.vaov,
            autoLoad: true,
            showControls: false,
            hfov: 100,
            yaw: props.yaw,
        });
        viewer.loadScene('newScene'); // this won't load anything up, even if I put in 'test'
        viewer.removeScene(oldScene);

      }

Am integrating it with a Vue3 project.

I'm sure Vue is doing something strange. As mentioned in the readme, frameworks like this aren't officially supported, because I have neither the time nor the motivation to try to debug these sorts of weird issues.
My guess is that there's probably some sort of asynchronous issue with props changing. You can try wrapping your scene configurations with structuredClone() and see if that helps, as it will create a deep copy.

authors: jerryComo mpetroff samedadl

repo: mpetroff/pannellum


text:
Error in GenerateDocumentation.php line 224:

When I run php artisan apidoc:generate I get the following error:
In GenerateDocumentation.php line 224: $comment = $reflection->getMethod($method)->getDocComment();
Method clientPay does not exist. Method clientPay actually exists on the affected controller as can be seen in the attached image:
  Route::post('/get-tutor/pay', 'PublicProfileController@clientPay')->name('get-tutor.pay');

I was able to get past the error with try-catch by doing the following:
try {
			$comment = $reflection->getMethod($method)->getDocComment();
			if ($comment) {
				$phpdoc = new DocBlock($comment);

				return collect($phpdoc->getTags())
					->filter(function ($tag) use ($route) {
						return $tag->getName() === 'hideFromAPIDocumentation';
					})
					->isEmpty();
			}
			return true;
		} catch (\Exception $exception) {
			echo "Error: " . $exception->getMessage() . ". ";
			return false;
		}

Turns out there's another controller of same name (difference namespace) that's doesn't have clientPay method defined. I installed laravel-swagger and it threw same error, but this time it included the controller namespace, name and method which made debugging easier.

repo: mpociot/laravel-apidoc-generator


text:
Miranda-based клиенты неправильно определяют транспорт

Из-за замены gc-1.0 на jabber.org/protocol/muc miranda-based клиенты принимают транспорт за сервер конференций.

8f370bfda4ba731ae3a76a4ce04a03882167cb7a

И как это проявляется?

Они хотят в него зайти, будто он конференция и не дают зарегистрироваться
On Nov 17, 2016 8:11 PM, "Yagiza" notifications@github.com wrote:

И как это проявляется?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/mrDoctorWho/vk4xmpp/issues/138#issuecomment-261242728,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADFP4sobE3gsnyIxmwxTGZakK-_QozK3ks5q_FJugaJpZM4K1NQy
.

Вот ради того, чтобы такие дебильные клиенты отсекать, и нужно делать всё по стандарту!

Мне, как разработчику, интересна поддержка большего количества клиентов. За
исключением случаев, которые я считаю абсурдными и которые не мешают
пользоваться транспортом.
On Nov 17, 2016 9:07 PM, "Yagiza" notifications@github.com wrote:

Вот ради того, чтобы такие дебильные клиенты отсекать, и нужно делать всё
по стандарту!
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/mrDoctorWho/vk4xmpp/issues/138#issuecomment-261254311,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADFP4lio9wVXKKotJ4konOmZcQDJbcj9ks5q_F8IgaJpZM4K1NQy
.

Такой подход имеет право на существование. Но к сожалению, он приводит к тому, что мы имеем: ни одного нормального XMPP-клиента: все как один имеют какие-то глюки и несовместимости.

@yagiza Как по-твоему клиент должен определять, чем является определенная нода на сервере? Он видит jabber.org/protocol/muc и решает, что это MUC. Вроде бы всё логично, нет?

Нет. Тип сущности определяется по элементам , а не по элементам  в disco#info.
Согласно XEP-0045, сервер конференций должен определяться по категории "conference" и по типу "text".
Элементы  нужны для того, чтобы определять, с какими фичами умеет работать данная сущность, а не для определения её типа. Все (нормальные) клиенты, поддерживающие работу с MUC тоже, публикуют эту фичу в disco#info, так что, миранда все контакты, сидящие на таких клиентах тоже, серверами конференций считает?

Все (нормальные) клиенты, поддерживающие работу с MUC тоже, публикуют эту фичу в disco#info, так что, миранда все контакты, сидящие на таких клиентах тоже, серверами конференций считает?

Насколько мне известно, это происходит только в обзоре сервисов.

Насколько мне известно, это происходит только в обзоре сервисов.
Ну а кто мешает в обзоре сервисов вбить полный жид контакта?

Пользователь скорее решит, что транспорт ему не нужен и уйдет, чем будет пытаться заставить его работать.

Вполне возможно. Другое дело, нужны ли тебе пользователи, сидящие в таких клиентах? Обычно с такими пользователями один геморрой.

authors: mrDoctorWho yagiza


text:
Big performance improvement

I've done these changes to improve the time to flash firmware.
I removed some small sleeps from the physical.py, and used the UPDI RSD flag to remove the need to receive acknowledgement bytes for every word of data written.
I also added a reset command.
I use a usb-serial adaptor; it all works for me at 115200 baud on the attiny1614 - but I haven't tried any other combination of serial port or device. I have only tested it under Linux.
It would be really useful to test this on other serial interfaces and devices, perhaps there is some reason why these changes will fail on different devices.
I am not an expert on UPDI or attiny microcontrollers, I just read the documentation :)

For me, the time taken to program my device was decreased from 72 seconds, to 12 seconds.

Looks good, @MarkR42! I will give it a run when i have time (hopefully soon).
The byte-by-byte write (with echo on each) was initially a workaround for a bug caused by having one stop-bit (which was of course fixed ages ago)
Turning ACK off is a more "advanced" change, but I think it should be safe at this speed.

Works for me using tiny416 with medbg's cdc on both mac and windows.
Programming time reduced from about 14s to about 4s - nice :)

Also tested good on windows on the m4809 using its own CDC, where the gain is even more significant - I suspect due to the nedbg's CDC being optimised for data frames (not bytes).  This is about where my test coverage ends :/

repo: mraardvark/pyupdi

license: MIT