Site request: Dish anywhere / sling player

Please follow the guide below

You will be asked some questions and requested to provide some information, please read them carefully and answer honestly
Put an x into all the boxes [ ] relevant to your issue (like that [x])
Use Preview tab to see how your issue will actually look like

Make sure you are using the latest version: run youtube-dl --version and ensure your version is 2016.07.13. If it's not read this FAQ entry and update. Issues with outdated version will be rejected.

[ x] I've verified and I assure that I'm running youtube-dl 2016.07.13

Before submitting an issue make sure you have:

[x ] At least skimmed through README and most notably FAQ and BUGS sections
[x ] Searched the bugtracker for similar issues including closed ones

What is the purpose of your issue?

[ ] Bug report (encountered problems with youtube-dl)
[ x] Site support request (request for adding support for a new site)
[ ] Feature request (request for a new functionality)
[ ] Question
[ ] Other

The following sections concretize particular purposed issues, you can erase any section (the contents between triple ---) not applicable to your issue

If the purpose of this issue is a bug report, site support request or you are not completely sure provide the full verbose output as follows:
Add -v flag to your command line you run youtube-dl with, copy the whole output and insert it here. It should look similar to one below (replace it with your log inserted between triple ```):
keybounceMBP:Keybounce michael$ youtube-dl -v "http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister
_6_p136571346?tab=dvr"
[debug] System config: []
[debug] User config: [u'-o', u'%(title)s.%(ext)s', u'-f', u'\nbest[ext=mp4][height>431][height<=719]/\nbestvideo[ext=mp4][height=480]+bestaudio[ext=m4a]/\nbest[ext=mp4][height>340][height<=431]/\nbestvideo[ext=mp4][height>360][height<=576]+bestaudio/\nbest[height>340][height<=576]/\nbestvideo[height>360][height<=576]+bestaudio/\nbestvideo[height=360]+bestaudio/\nbest[ext=mp4][height>=280][height<=360]/\nbest[height<=576]/\nworst', u'--write-sub', u'--embed-subs', u'--recode-video', u'mp4', u'--mark-watched']
[debug] Command-line args: [u'-v', u'http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr']
[debug] Encodings: locale UTF-8, fs utf-8, out UTF-8, pref UTF-8
[debug] youtube-dl version 2016.07.13
[debug] Python version 2.7.5 - Darwin-13.4.0-x86_64-i386-64bit
[debug] exe versions: ffmpeg 3.1.1, ffprobe 3.1.1, rtmpdump 2.4
[debug] Proxy map: {}
[generic] marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr: Requesting header
WARNING: Falling back on generic information extractor.
[generic] marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr: Downloading webpage
[generic] marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr: Extracting information
ERROR: Unsupported URL: http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr
Traceback (most recent call last):
  File "/Users/michael/bin/youtube-dl/youtube_dl/extractor/generic.py", line 1565, in _real_extract
    doc = compat_etree_fromstring(webpage.encode('utf-8'))
  File "/Users/michael/bin/youtube-dl/youtube_dl/compat.py", line 2525, in compat_etree_fromstring
    doc = _XML(text, parser=etree.XMLParser(target=_TreeBuilder(element_factory=_element_factory)))
  File "/Users/michael/bin/youtube-dl/youtube_dl/compat.py", line 2514, in _XML
    parser.feed(text)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/xml/etree/ElementTree.py", line 1642, in feed
    self._raiseerror(v)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/xml/etree/ElementTree.py", line 1506, in _raiseerror
    raise err
ParseError: not well-formed (invalid token): line 40, column 724
Traceback (most recent call last):
  File "/Users/michael/bin/youtube-dl/youtube_dl/YoutubeDL.py", line 681, in extract_info
    ie_result = ie.extract(url)
  File "/Users/michael/bin/youtube-dl/youtube_dl/extractor/common.py", line 347, in extract
    return self._real_extract(url)
  File "/Users/michael/bin/youtube-dl/youtube_dl/extractor/generic.py", line 2282, in _real_extract
    raise UnsupportedError(url)
UnsupportedError: Unsupported URL: http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr

If the purpose of this issue is a site support request please provide all kinds of example URLs support for which should be included (replace following example URLs by yours):
Typical playlist (all recorded episodes of a series):
http://www.dishanywhere.com/shows/the_late_show_with_stephen_colbert_p136124004?tab=dvr
http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=dvr
All on-demand episodes of a show
http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346?tab=ondemand
Typical specific episodes:
On demand: http://www.dishanywhere.com/shows/marvels_ultimate_spiderman_vs_the_sinister_6_p136571346/episodes/4758048/2112288/
Normal recordings:
http://www.dishanywhere.com/sling/8008/16789505
NB: This site requires a login, obviously. Less obviously: recordings will either wind up redirecting to local network (if you can talk directly to the recorder) or passing through dish's server (if you are not at home -- firewalls prevent direct contact with the receiver).

Description of your issue, suggested solution and other information
Explanation of your issue in arbitrary form goes here. Please make sure the description is worded well enough to be understood. Provide as much context and examples as possible.
If work on your issue required an account credentials please provide them or explain how one can obtain them.

If you need to use my tv provider information to do this, let me know. (Is that what the tag means?)


Include case reducers in createSlice result

Per discussion at https://github.com/reduxjs/redux-starter-kit/issues/82#issuecomment-533877434

@phryneas , @denisw , @Dudeonyx , @jessidhia : can I get some eyes on this to make sure I didn't do anything really stupid?
The unit tests and type tests still pass, and it looks like everything is being inferred correctly in the output.

@phryneas :

It needs the round-trip to something with the initial reducer types, or it just accepts anything.

Not sure I quite understand what you're saying here.  Can you clarify?

Awright, I think I actually have a working solution here!
I added the two type tests suggested by @phryneas , and attempted to implement the suggested solution ( https://github.com/reduxjs/redux-starter-kit/pull/209#discussion_r332356920 ).
I fixed the "can't have a required type after a default type" error by swapping the order of the two generics in Slice.
From there, I tried to set up some kind of mapping, loosely based on the CaseReducerActions type.  My first couple attempts got the caseReducers.nonExistent test failing as desired, but the mismatched payload test was incorrectly passing without error due to the payload being inferred as any:
type ExtractPayloadActions<T> = T extends SliceCaseReducerDefinitions<
  any,
  infer PA
>
  ? PA
  : never

type OnlyCaseReducers<
  CaseReducers extends SliceCaseReducerDefinitions<any, any>,
  State = any
> = {
  [Type in keyof CaseReducers]: CaseReducer<
    State,
    ExtractPayloadActions<CaseReducers>[Type]
  >
}

I eventually figured out that it was basically falling back to a default PayloadAction<any> somehow.
That clued me in that I need to extract the specific action type for this situation.  Inspired by the PayloadForReducer type, I came up with this:
type ActionForReducer<R, ActionType = Action<string>> = R extends (
  state: any,
  action: PayloadAction<infer P>
) => any
  ? PayloadAction<P>
  : ActionType

type OnlyCaseReducers<
  CaseReducers extends SliceCaseReducerDefinitions<any, any>,
  State = any
> = {
  [Type in keyof CaseReducers]: CaseReducer<
    State,
    ActionForReducer<CaseReducers[Type]>
  >
}

That got both the "mismatched payload" and "nonexisting field" type tests throwing errors as expected.
I'm not sold on the name OnlyCaseReducers, but I'm open to suggestions.

You're on a good way here, but not quite there yet (and those types are complicated, so every little bit of frustration you might be feeling is perfectly understandable!).
The important thing here is to identify why your ExtractPayloadActions type or ActionForReducer might fail.
First your ExtractPayloadActions type: in theory that's perfectly fine! But the SliceCaseReducerDefinitions type itself is unfortunately so complex, that TS doesn't manage fitting "the right thing" into the Generic any more (at least that's how I explain it to myself, others to the rescue).
In your ActionForReducer, you're doing exactly the right next step: when TS can't figure it out, you need to be more specific. And for the first part, that does look good to me. The "else" part needs a bit works still, though.
Take a look at potential shapes that could be passed to the R parameter.
The default shape is the one you already identified: (  state: any,  action: PayloadAction<P>) => any
But then you're just falling back to ActionType - and as ActionType has no further mentioning in the type, it can't be inferred by TS and will always be it's default value Action<string>.
But there is another shape you can test for that we definitely know of:
{
  reducer(state: any,  action: PayloadAction<P>): any
}

so you can do a second nested ternary there, testing for that type and inferring P from there.
As for the "does not match any of the above" branch, I would not go for your ActionType or any Action, but just for never, void, unknown or the likes. If it doesn't match those, we're in a case that should never happen, so there's no need for a sensible fallback, in contrary: that fallback might prevent us from catching bugs if it isn't abviously unfitting.
I would suggest you add another test for the case with an enhanced reducer (including expect-error tests like I wrote above) - and then you're really super close :)

@phryneas : thanks for coaching me through this!  As I said, part of me just wants someone to fix all this and make it work so we can push RSK up to 1.0 ASAP, but at the same time I really need to start becoming not-totally-incompetent with this complex type stuff.
I'll try to tackle your suggested directions myself and see what happens.

@phryneas : had to read through your instructions a couple times, but turns out you were really walking me through this one, weren't you :)
(I recognize the "tell the clueless n00b the exact steps they need to do, politely" phrasing.  Goodness knows I've done that enough times myself.)
Added the additional type test, implemented the ternary, and saw that test start failing as expected.
From there, I cleaned up a few of the types in createSlice a bit, and decided to rename OnlyCaseReducers to SliceDefinedCaseReducers.  (There are entirely too many uses of the words "Slice", "Case", and "Reducers" in this file already, but oh well!)
How's this look now?

A little bit - but in the end, you did it, with all the failures that come along on the way.
Just a little secret: for the reducer with payload stuff, I wrote that whole thing at least four times from scratch, completely resetting because something was not behaving as I wanted and I was going routine-blinded with my existing code. Wanted to spare you of that frustration, because it is not a learning one ;)
The code looks good now, I've added two more small comments, but I believe you did it :)
It's early in the morning here and I haven't had my coffee yet, so I'll do a second round of review in the evening, just in case I missed something.

Awright, let's do this!


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?

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://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>:8877/5e5412c2765c9bd/web/viewer.html is of course not remembering the "Grid scrolling" setting).
With the new version ( http://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>: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://<IP_ADDRESS>: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.


Feature request: Possibility of asymmetric session encryption

Hi, I think it would be cool to be able to asymmetrically encrypt passwords and sessions when intercepting. This is necessary when evilginx is deployed on vps, which can be compromised or is not trusted. What do you think, @kgretzky?

Any movement on this? I would love to be able to have the gathered data saved to an encrypted file rather than cleartext, or an encrypted email sent to me for each submission.


Wrong word on page

Step 4 in the Render lists information section: "Things to note about hostType in the _renderListAsync method:"
The word "hostType" should be "EnvironmentType"?

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 71e8ee66-fe7c-a46e-eeec-fb199962042c
Version Independent ID: f78aef17-5aa2-8807-73d4-509faecbd153
Content: Connect your client-side web part to SharePoint (Hello World part 2)
Content Source: docs/spfx/web-parts/get-started/connect-to-sharepoint.md
Product: sharepoint
Technology: sharepoint-framework
GitHub Login: @spdevdocs
Microsoft Alias: spdevdocs

Thank you for the issue.
Would you be willing to submit a PR for this change?

Ok I've created a PR for it. I hope I've done it write. I'm very new to GitHub. :)

Oops...hope I've done it right.

Unfortunately not, you just made the update in your fork. You need to submit a PR to request it to be merged... might want to google how to submit a PR. There are a TON of resources on GitHub and videos on YouTube showing this.

@ConaxLiu refer to how I did it in the above links... the only diff: you can't merge your PR once you submit it.

Hi @andrewconnell, on the bottom of every page has this notice: "If you find an issue in the documentation or in the SharePoint Framework, please report that to SharePoint engineering by using the issue list at the sp-dev-docs repository or by adding a comment to this article. Thanks for your input in advance."
That's the reason why I had left comment. I don't even know why I need to try to update the page and submit PR myself. If that's what people need to do to correct possible mistakes on these pages, then the above mentioned notice is rather misleading and should be updated to point readers to the information of exactly how they can contribute properly.


Typo :  "/_apiimport"

You guys do have tests, don't you
blahblahblah
body:
      { error: true,
        code: 404,
        errorNum: 404,
        errorMessage: 'unknown path \'/_apiimport?type=auto&collection=posts\'' },

Win7 64bit
Fixed after downgraded to<EMAIL_ADDRESS>

Seems like the path prefix is wrong: should be /_api/import instead of /_apiimport.

Does this patch #233 fix the issue?

I doubt that. But might the same line of code be responsible and need to be
changed to this?
this._api = this._connection.route('/_api/')

Note that I also fixed this one:
https://github.com/arangodb/arangojs/commit/152cf84847befb156f8be2c8d73792686c75de72
However, I am unable to create a new driver release. @pluma has been notified to do that.

I've made a bugfix release with these changes.
@ctmakro as you can see from the README we do have some test coverage though there are quite a few known blind spots. I would like to apologize that the latest minor release (4.5) was not quite up to the usual standard however and that there has been a bit of a delay in handling the issues introduced in that version.
We had to write and publish the release within less than a day for internal reasons and it seems some of the optimizations were not quite as straightforward as they seemed to be (resulting in string concatenation problems for less covered functionality like the one you encountered). I was unavailable for the days after the release, so I couldn't address the issues myself (because npm currently only offers organizations for commercial accounts we previously did not have other maintainers authorized to publish releases).
However if you do encounter other problems with this minor release (4.5), the only difference between it and the one immediately preceding (4.4) it are the optimizations. You can install that version of arangojs using npm install<EMAIL_ADDRESS>instead of the usual npm install arangojs.

@pluma I found this bug several days ago when exporting from MySQL then importing to ArangoDB (yes we changed our entire infrastructure, now based on Arango). After initial deployment of our project with npm install on our production server, the problem occured.
Sorry for my language, not really trying to blame anyone.


Cannot format SD Card / non stable access card (1.51 - RG405V)

I have a 1tb SD card.

Every time I try to format it, the system crash. and goes into in a very slow boot mode ~5 minutes loop with the "G" logo
I successfully (kind of) was able to format it once but then 90% of the case it is not recognized and the system asked me to reformat it => it can be fix via putting the sd card in a PC, trying to repair it and putting it back in the console

Thank you

Did you try formatting it on your PC to exFAT?

Closing due to inactivity.


feat(macos): add tabbing_identifier option, closes #2804, #3912

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Docs
[ ] New Binding issue #___
[ ] Code style update
[ ] Refactor
[ ] Build-related changes
[ ] Other, please describe:

Does this PR introduce a breaking change?

[ ] Yes, and the changes were approved in issue #___
[x] No

Checklist

[x] When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
[x] A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
[ ] I have added a convincing reason for adding this feature, if necessary

Other information
Closes #2804, closes #3912
Uses the new APIs introduced in Tao in https://github.com/tauri-apps/tao/pull/586 and https://github.com/tauri-apps/tao/pull/592.

Is it just the Windows/Linux user in me or is this something that should be off by default (or always)?

I'd love it to be off by default but that's not default behaviour on macOS…
Should we override it by default? I don't know. 🤔

Just a thought – if it's off by default that will be a breaking change, so even if we do want that, maybe best to add it as-is for v1.2 and then change the default in v2.0?

electron actually uses tabbingIdentifier and disables tabbing if the window is transparent or if it doesn't have decorations.
if (tabbingIdentifier.empty() || transparent() || !has_frame()) {
    [window_ setTabbingMode:NSWindowTabbingModeDisallowed];
  } else {
    [window_ setTabbingIdentifier:base::SysUTF8ToNSString(tabbingIdentifier)];
  }

This also closes #2804

Ah, I was sure there was another issue, but I failed to find it, thanks.

electron actually uses tabbingIdentifier

I did try that it it didn't seem to have any effect, but I might have done something wrong. Might be worth playing with it again.

and disables tabbing if the window is transparent or if it doesn't have decorations.

That seems sensible. Though I must say I'm quite drawn to the idea of disabling it by default, and providing an option to enable. What do you think?

I can also play with the tabbingIdentifier later - I expect it to define groups of windows that can be tabbed together. I like the idea of disabling it by default, and it's what electron does.

I've opened a PR to add tabbing identifier support on tao. I couldn't understand how it works when changing the identifier at runtime, but the builder with_tabbing_identifier method works fine. I think we should include it here and work similar to electron, they have that nice logic for it.

Apologies, now I look properly, I see that's effectively what you've done.
Seems good, but I'll go over it properly now 😄

Looks good! Thanks @lucasfernog.


read k8s configmap error

I got the error message below :
panic: runtime error: index out of range

goroutine 1 [running]:
github.com/micro/go-plugins/config/source/configmap.split(...)
	/Users/anakin/Documents/go/pkg/mod/github.com/micro/go-plugins@v1.2.0/config/source/configmap/util.go:53

my configmap yaml:
apiVersion: v1
kind: ConfigMap
metadata:
  name: test-config
  namespace: default
data:
  log.level: WARNING
  log.location: LOCAL

Please help me~~~

data:
log.level: WARNING
log.location: LOCAL
should be like this
data:
log: level=WARNING\nlocation=LOCAL
my usuage now:
data:
projectconf: registerlimit=1000
registry: enablek8s=0

@Linnkenn Thank you for your reply!

I'm assuming this has now been resolved so I'm closing this.


Move stress root-code to different submodule

The functions from the original stress repository could be moved to a submodule of their own, next to _spherical_harmonics. This could make some of the downstream developments much easier. Similarly there should be a separate submodule for measurements (#98 ) and visualization (#99)

Started working on it here


SyntaxError: Named export 'Arrow' not found.

I am getting this issue please let me know how can I fix this.
SyntaxError: Named export 'Arrow' not found. The requested module '@radix-ui/react-tooltip' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:
import pkg from '@radix-ui/react-tooltip';
const {Provider: $TBcmg$Provider, Arrow: $TBcmg$Arrow, Root: $TBcmg$Root, Trigger: $TBcmg$Trigger, Content: $TBcmg$Content} = pkg;

We were forced to switch from cjs to esmodules due to wagmi and ever since then we needed a workaround for nextjs. Here's the guide on how to add it:
https://docs.reservoir.tools/reference/troubleshooting#nextjs-commonjs-module-error
It's a pretty small and simple change.

Thanks, it works.
Please add this to the bellow page as well.
https://docs.reservoir.tools/docs/reservoirkit-ui#configuring-reservoirkit-ui


Adding new Trusted Profile Analyzer QSG

This PR adds two new Quick Starts guides for the Service Preview release of the Trusted Profile Analyzer (TPA):

tpa-searching-for-vulnerability-information
tpa-scanning-an-sbom-file

These are to be added to the Learning Resources section under Application and Data Services.
Fixes: Jira TC-332
@Hyperkid123 @ryelo : I've opened Jira task RHCLOUD-29959 for adding these QSG to HCC.  Please don't hesitate to yell at me if you have any questions.  Thank you for your help!

/ok-to-test

You are also missing the color attribute in the content that is used for the badge in the UI.


How can I create Stringdata from data?

Hello,
Im using argocd, and I want to configure github repos and with argo 2.1 we should create secrets, but instead of data, this secrets has stringData.
Like this:
apiVersion: v1
kind: Secret
metadata:
  name: private-repo
  namespace: argocd
  labels:
    argocd.argoproj.io/secret-type: repository
stringData:
  url: https://github.com/argoproj/private-repo
  password: my-password
  username: my-usernam

I tried to have the same approach using external secrets
apiVersion: "kubernetes-client.io/v1"
kind: ExternalSecret
metadata:
  name: github-repo-token
  namespace: argocd
spec:
  backendType: systemManager
  data:
    - key: github_repo_token
      name: token
    - key: github_repo_token_user
      name: username
  template:
      metadata:
          labels:
              argocd.argoproj.io/secret-type: repository
      stringData:
        url: url_repo
        password: data.token
        username: data.username

Is it right? Because didnt work.
Anybody could help me with that?
Thank you

You can use the lodash templates in the stringData like <%= data.token %> and  <%= data.username %>

@rubenssoto Have you made any progress on this?  We're attempting to do the same thing as you are but for the life of me I can't figure out how to get the stringData field in the native Kubernetes secret to populate.

Hey @kencieszykowski, I don't know if you resolved your issue but I had a problem with adding a cluster to ArgoCD declaratively, just like @rubenssoto with adding a repository. I thought that the secret must contain the stringData but it seems it can use data as long as you base64 encode the secret as the stringData should be.
The problem I had was that external-secrets when retrieving the secret put all the secret information into one key, i.e if you have this:
...
data:
    - key: /secret/path
      name: secretConfig

Your secret information will all be inside of the secretConfig, i.e this would create the k8s Secret with the following config:
...
data:
  secretConfig: eyJhd3NBdXR.....

And this won't work since the stringData expects to have different configuration, i.e key - value pairs. That's why you need to use the stringData templating, to get the right Secret configuration.
In my case I had to had the following stringData, check the docs for more info (https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#clusters):
...
stringData:
  name: 'some-name'
  server: 'some-server'
  config: |
    {
      "login-config": {}
    }

So what I did was I added my secret config in AWS secrets manager as a JSON, i.e I converted the stringData - name: ... into JSON and I put it like that into a secret. I did this because the external-secrets controller uses lodash templating.
{
  "name": "some-name",
  "server": "some-server",
  "config": {
    "awsAuthConfig": {
      ...
    }
  }
}

So this was the external-secrets config I made to get the right config:
template:
    stringData:
      name: <%= JSON.parse(data.clusterConfig).name %>
      server: <%= JSON.parse(data.clusterConfig).server %>
      config: |
        <%= JSON.stringify(JSON.parse(data.clusterConfig).config) %>
data:
    - key: /secret/path
      name: clusterConfig

And this resulted in creating the secret with:
data:
  name: eyJhd3NBdXR.....
  server: eyJhd3NBdXR.....
  config: eyJhd3NBdXR.....

I too had stumbled on the issue, it would be great to have stringData field by default but till then this is my workaround :)
--> secret in AWS Secrets Manager as a json
{
  "username": "<my_username>",
  "password": "<my_password>"
}

--> external secret
---
apiVersion: kubernetes-client.io/v1
kind: ExternalSecret
metadata:
  name: vikas
  namespace: argocd
spec:
  backendType: secretsManager
  region: ap-southeast-2
  template:
    stringData:
      name: codecommit
      type: git
      url: "https://git-codecommit.ap-southeast-2.amazonaws.com/v1/repos/<myrepo>"
  data:
  - key: arn:aws:secretsmanager:ap-southeast-2:111111111:secret:<my_secret>
    name: username
    property: username
  - key: arn:aws:secretsmanager:ap-southeast-2:111111111:secret:<my_secret>
    name: password
    property: password

--> secret created
apiVersion: v1
data:
  name: Y29kZWNvbW1pdA==
  password: SkdG....
  type: Z2l0
  url: aHR0c....
  username: Y25zb....
kind: Secret
metadata:
  name: vikas
  namespace: argocd
type: Opaque


Removed note about Content camera support for Surface Pro-based system

Content camera support for Surface Pro-based system (Minimum required app build: <IP_ADDRESS>)

@JohanFreelancer9 Copy editing is needed for this PR. Thanks.

@kenwith PR has been copyedited and is ready for final review, could you please check and merge? Thanks!


improvementOptimize drop stats operation.

Before, drop stats operation need to call columns * followers times of isMaster() function and the same times of rpc to drop remote column stats. This pr is to reduce the rpc calls and use more efficient way to check master node instead of using isMaster()
backport https://github.com/apache/doris/pull/30144
If this is a relatively large or complex change, kick off the discussion at<EMAIL_ADDRESS>by explaining why you chose the solution you did and what alternatives you considered, etc...

run buildall

run buildall


Hot reload improvements

show the next error fully after a hot reload
hide _AssertionError in stacks
immediately rebuild after a reassemble, so that hit tests work
catch errors when notifying global key listeners


Added changes to Terraform Equinix Modules docs to include versioning, pr's, and commits

This PR contains the following changes:

hyperlink format changes
markdown lint throughout the doc
added info in section 5. and 6. of the Using the Terraform Equinix Modules Template section

@ctreatma I have updated the Terraform md doc slightly to include the CI jobs that will run once a PR is submitted and the CI jobs that will run when the PR is approved and merged.


upgrade: With skip_puppet, show what puppet changes are outstanding.

This prevents puppet changes from building up over time.

This is great, merged, thanks @alexmv!


Add field name for left- and right-hand side of declarations

Given all Talon declarations have the form foo: bar, it would be nice if there were a consistent field for foo and bar.  I could see how you might want more specific field names for those, which is fine, but at least having a field name for all of them would make it easer to work with the grammar

Maybe? Cc/ @phillco @andreasarvidsson

sgtm!

Sounds good