CSSOM View document.scrollingElement

The Blink team would like a TAG review of document.scrollingElement before LGTM-ing shipping it.
https://groups.google.com/a/chromium.org/d/msg/blink-dev/mJ5SlCj3CHc/hkQuqROnwl4J
https://groups.google.com/a/chromium.org/d/msg/blink-dev/mJ5SlCj3CHc/SnJ5rNxCiOUJ
Spec: http://dev.w3.org/csswg/cssom-view/#dom-document-scrollingelement
Background/rationale
https://github.com/operasoftware/devopera/issues/242
https://lists.w3.org/Archives/Public/www-style/2015Apr/0108.html
Tracking bugs
https://code.google.com/p/chromium/issues/detail?id=475531
https://bugzilla.mozilla.org/show_bug.cgi?id=1153322
https://bugs.webkit.org/show_bug.cgi?id=143609

The next branch point is on May 15th, 2015, so it would be very good to have any feedback well before that in case we want to ship with the next milestone.

I'm personally unclear why this is the right path. Given the existence of e.g. https://github.com/mathiasbynens/document.scrollingElement it seems like we are essentially baking a jQuery-style browser normalizer ... into the browser. Which means we'll have to support it going forward forever.
Why does this drop-in replacement need to be provided by the browser? It's marginally easier for authors, but is it worth the long-term cost of adding cruft, in the future where document.scrollingElement just becomes a synonym for document.documentElement? You have to consider the cognitive burden for future authors who read code and see document.scrollingElement, then have to go look up what that does, instead of just seeing document.documentElement and knowing immediately what it does. (And yes, it's not just a synonym in quirks mode, but I don't think adding a new API for quirks-mode users is really that interesting).
I also don't understand how we expect this to work out. Is the envisioned sequence something like this?

spec says document.scrollingElement === document.documentElement
Blink implements document.scrollingElement === document.body; Gecko implements document.scrollingElement === document.documentElement
Hopefully nobody writes code that assumes one or the other (e.g. uses document.scrollingElement.firstChild or document.scrollingElement.addEventListener(...)), or uses UA-sniffing such that different code paths within the same app depend on the different behaviors for document.scrollingElement
We try switching to standards-compliant behavior?

I am worried about the "hopefully" here, as it just seems to set us up for repeating the cycle. Likely I am missing something.

I agree there is some cause for concern here.  I believe your argument is only compelling when your ignore quirks mode.  Even in the future when Blink/WebKit are fixed, libraries still need a bit of logic to determine which element is the scrolling element.
Trying to get sites to update to support the spec compliant behavior without a UA check it'be become clear to me that scrollingElement is a big help in practice.  Facebook made use of such an API internally, was unwilling to do the (somewhat intrusive iframe-measurement) hack and so had to make a fairly substantial change to their codebase to follow separate get ('||' the two values) and set paths (set on both elements).  Closure is even worse - their public API has 'goog.dom.getDocumentScrollElement'.  We course of course could argue that they (and anyone else like them) should use an iframe measurement hack (like the above polyfill - which by the way still has issues), or they should deprecate this public API.  But that would take way more debate/effort than at least I'm willing to stomach.
If we think the value of document.scrollingElement isn't high enough, then that's OK with me.  I'll give up trying to ever fix this wart in blink and be resigned to either the status quo (continuing the LONG tradition of violating the CSS spec here) or trying to convince the spec and other browsers to change  to bake WebKits bug into their design (thereby making body-scrollers confusingly unavailable to JavaScript).
So to me the choice of long term future seems to be between:

All browsers eventually behave the same and we have a somewhat questionable tiny additional API
All browsers have irrational behavior where body doesn't truly behave like an element, and documentElement controls scrolling via some APIs (CSS overflow) and not others (scrollTop)
Status quo - confusing differences between browsers

I'd be happy to be proven otherwise.  There have been several failed attempts over the years to fix this wart, but adding a new API to ease the transition is the one thing that hasn't been tried.  Perhaps I'm just crazy for trying again to solve this (after IE told me they were forced to copy our bug in EdgeHTML).

I believe your argument is only compelling when your ignore quirks mode. Even in the future when Blink/WebKit are fixed, libraries still need a bit of logic to determine which element is the scrolling element.

The scenario here is a library that intends to work in quirks mode as well as standards mode? Is that a widespread need? If so that would indeed be a good argument.
The rest of your post explains why you think this will work but doesn't really address my concern for why I think it could not work. In other words, I think you are missing long-term futures like

All browsers have irrational behavior where body doesn't truly behave like an element, and documentElement controls scrolling via some APIs (CSS overflow) and not others (scrollTop), and browsers also have a scrollingElement alias for body

or

Status quo - confusing differences between browsers - including scrollingElement now also being a new difference

IE told me they were forced to copy our bug in EdgeHTML

Ouch, so this has already happened. Did they tell you which kinds of sites were fixed by this? Is it just sites that sniff for WebKit, or is it more intricate?

seems like we are essentially baking a jQuery-style browser normalizer ... into the browser. Which means we'll have to support it going forward forever.

Isn't this how browser APIs evolve to some extent?  As frameworks figure out better API surface area to reduce developer confusion and interop problems, we should consider adopting it into the core platform for universal availability?

The scenario here is a library that intends to work in quirks mode as well as standards mode? Is that a widespread need? If so that would indeed be a good argument. Are libraries currently using tricks like those on display in the polyfill before they use scrollTop, in order to cater to quirks mode?

I don't know how common this is.  Many of the solutions I've seen for this "how do I identify the scrolling element" question involve checking for quirks mode.  Eg. see Closure's code here: https://github.com/google/closure-library/blob/32365aba43acb36c5d693256ef5d4dbe3bddddfe/closure/goog/dom/dom.js#L632
From what I can see, jQuery deals with this issue by creating unified scrollTop/scrollLeft wrapper APIs which use the window api when appropriate (eg. window.pageXOffset) rather than attempt to use body/documentElement at all.  Eg: https://github.com/jquery/jquery/blob/74ae5444832b2fb966768a97281d2ad8c088bc58/src/offset.js#L170.

I think you are missing long-term futures like ...

Yes, I agree those outcomes are a risk and I don't have any concrete data / arguments to counter it.  My instinct is that the cost (complexity/confusion) of a precisely defined but extraneous scrollingElement API is relatively low compared to the cost of the other options.  Also that it's worth taking some small breakage to fix this issue, and therefore the benefit (over a suitably long time horizon) outweighs the risk that we'll never fix it.
But I don't see any obvious way around this being a judgement call.  It sounds like Safari agrees with your judgement (same sort of argument), while Mozilla, IE and the spec editor's judgement is that it's worth it.  I'm happy to defer to your judgement if TAG should be the one to make the judgement call here.

Ouch, so this has already happened. Did they tell you which kinds of sites were fixed by this? Is it just sites that sniff for WebKit, or is it more intricate?

From what I've heard, it's only due to UA sniffing.  EdgeHTML includes 'Webkit' in it's UA, so in general plans to copy WebKit bugs where it matters for interop and we don't have a clear plan to solve them.  My hope was that we could still solve this before it gets worse - if I can make the change stick in blink then EdgeHTML and WebKit would hopefully find it relatively easy to follow suit.

Some more background on why moving this from user code to the browser might be a good thing: feature-detecting the scrolling element (especially if you need to get the edge cases right, which is what libraries usually aim for) is costly. One way is to actually scroll the document:
var $scrollElement = (function() {
  var $html = $(document.documentElement);
  var $body = $(document.body);
  var bodyScrollTop;
  if ($html.scrollTop()) {
    return $html;
  } else {
    bodyScrollTop = $body.scrollTop();
    // Scrolling the body doesn’t do anything.
    if ($body.scrollTop(bodyScrollTop + 1).scrollTop() == bodyScrollTop) {
      return $html;
    } else {
      // We actually scrolled, so undo it.
      return $body.scrollTop(bodyScrollTop);
    }
  }
}());

The terribleness might explain why so many (e.g. Google, Facebook) opt to UA sniff instead, which is more performant but leads to compat problems (@rbyers knows all about them).

This originally surfaced as a bug in jQuery/jQuery UI. IMO, scrollingElement is just another hack around the issue. The canonical, non-UA sniff, AFAIK, is to do $html.scrollTop() || $body.scrollTop(). The problem was that lots of people were using $html.scrollTop() + $body.scrollTop(), which is problematic when the body or the html element have overflow: scroll; applied.

Mike, do you have a suggestion for how we get frameworks like closure to update without adding an API like scrollingElement?  I'm hearing a bunch of objection to scrollingElement, but I haven't yet heard anyone suggest a realistic path for actually fixing the problem here.  As with most compatibility vs. rationality debates the problem is over constrained.  If we continue to hold out for a perfect ideal future, we'll remain at our currently horribly confusing state IMHO.

The bug was inherited from Webkit, who has since ripped the bandaid off.
If the future is that scrollingElement is just another stop gap, and already has a polyfill, we point the libs to the polyfill until they can wrap their heads around just always using document.documentElement.
jQuery has worked around this in UI for a while. In fact, we're eagerly awaiting the bandaid to be ripped off simply so we can remove our workarounds.
Perhaps I'm misunderstanding, but the UA sniffs and the polyfills are all because Blink hasn't bit the bullet yet.
Shipping scrollingElement won't alleviate need for polyfill of old blink, so nothing is gained by introduction of the new API?

Wait, what do you mean WebKit has ripped the bandaid off?  There has been an open bug there for 9 years (this and previously this) which they've tried and failed to fix (for similar reasons as us).  If they successfully changed then I have no doubt we could too.  But instead the opposite is happening - IE has switched to using 'WebKit' in their user agent and due to compat issues felt they had to also copy this bug.
Shipping scrollingElement will alleviate the need for an awkward polyfill - sites are free to fall back on the simple UA check they're already doing today when scrollingElement isn't defined.  scrollingElement gives us a simple drop-in upgrade path for such sites.

I missed that Webkit had to revert. I was eagerly watching it land initially. My apologies for the mistake.
It still just seems like a weird wart to me to have to add API surface simply for feature detecting the presence of a bug.
In either case, I think having a canonical polyfill is good enough, but for me, anything we can do to move the web forward here works.
I'm tired of writing $('html, body').scrollTop(0); to get back to the top of the page in a cross browser way.

Domenic, please take a look at the current (now believed to be correct) implementation of the scrollingElement polyfill: https://github.com/mathiasbynens/document.scrollingElement/blob/master/scrollingelement.js.
Hopefully the complexity / subtlety here helps justify why it'll be a tough sell to get frameworks/apps to fix their bug by replacing a 1-line UA check with this 100-line polyfill.

To be fair, more than a few of those lines are to support IE4 and IE7. Also a few of the lines ensure its a property rather than a function that can be called. Also, if I'm reading it right, it always returns document.documentElement is non-quirks mode?
Does FB or Closure support non-quirks mode or any of those effected by this bug actually support the browsers that take up a good portion of the logic?

Also, if I'm reading it right, it always returns document.documentElement is non-quirks mode?

Not in buggy WebKit/Blink browsers.

Does FB or Closure support non-quirks mode or any of those effected by this bug actually support the browsers that take up a good portion of the logic?

Closure at least supports quirks mode - yes.  Can't say about facebook.

@RByers I really just don't think it makes sense to ship that kind of polyfill with a browser. You've found one library (a Google library!) which directly exposes the scrolling element. Because they've purposefully added such complexity, it seems totally reasonable for them to eat it and ship the few lines necessary for non-IE4/IE7 support. Others can use much simpler tricks that have been cited in this thread. That said, I'm not the one on the ground here, and if you really think this'll make a difference (although IMO Closure really should not count), then maybe I'm just wrong.
Anyway, I think I've expressed my opinion enough on this thread, especially since I'm just ex-TAG. We'll let others chime in.

It might be worthwhile to point out that a native implementation does not have the same complexity as the polyfill. The "scrolling element" is a concept that the browser needs anyway to implement scrollTop et al, so it's just a matter of exposing that concept with an IDL attribute.

The “scrolling element” is a concept that the browser needs anyway to implement scrollTop et al, so it’s just a matter of exposing that concept with an IDL attribute.

This makes it sound like a no-brainer, given the Extensible Web Manifesto.

So, just so that I'm clear, can you show a code example of how FB or jQuery or Closure uses this property, including cases where it isn't present?

With polyfill:
<!-- in <head> if necessary, otherwise before </body> -->
<script src="scrollingelement.js"></script>

document.scrollingElement.scrollTop = 100;

without polyfill, fall back to UA check
function getScrollingElement() {
    if ("scrollingElement" in document) {
        return document.scrollingElement;
    }
    return /* whatever UA check they use now */;
}

getScrollingElement().scrollTop = 100;

I've talked with the Closure developer for this today.  He is unwilling to use the iframe measurement hack as it's too expensive.  I.e. scrollingElement or something like it appears to be necessary to get most Google properties updated.

@RByers Again, I'm a layman, so please just take my comments as feedback. I've also been trying to solve this problem for a while on behalf of jQuery, and this is as close as we've ever gotten to a solution....
what about a boolean property rather than an additional reference to document.documentElement? I think @domenic and my (layman) main objection is about yet another property referring to the same underlying element. Perhaps document.documentElement.isScrollingElement?
This would look like:
function getScrollingElement() {
    if (document.documentElement.isScrollingElement) {
        return document.documentElement;
    }
    return /* whatever UA check they use now */;
}

You'd also have the benefit of querying an element to see if it is the scrolling element without having to compare it to another element:
function isScrollingElement(element) {
  if ("isScrollingElement" in element) {
    return element.isScrollingElement;
  }
  return /*some UA check */;
}

Sure, that would be fine with me as well.  You could argue that adding to 'Document' is a smaller change than adding to 'Element', but then we could have 'boolean Document.bodyScrollsDocument' or some such.  I'm happy to take any suggested tweaks like that to the CSS WG and other vendors.

@mikesherov

what about a boolean property rather than an additional reference to document.documentElement?

It’s not just another reference to a fixed value. document.scrollingElement would only be a reference to document.documentElement in standards mode and if the browser implements the scrolling element as per the spec. I.e. if WebKit were to implement document.scrollingElement before fixing their bug, document.scrollingElement would not point to document.documentElement even in standards mode.

[…] This would look like:
function getScrollingElement() {
  if (document.documentElement.isScrollingElement) {
    return document.documentElement;
  }
  return /* whatever UA check they use now */;
}

No, it would then have to look like:
function getScrollingElement() {
  if (document.documentElement.isScrollingElement) {
    return document.documentElement;
  }
  if (document.body.isScrollingElement) {
    return document.body;
  }
  return /* whatever UA check they use now */;
}

Discussed at SF F2F; Alex will draft a review.

@mathiasbynens sure, that makes sense. Thanks for walking me through it. I'll state my case one last time, and then be done with it :-)
I suppose that my main objection is simply that I'd prefer to just feature detect that this bug is fixed, and go on my merry way of behaving the way the spec is written, rather than augmenting the spec to including an additional reference (even though it changes based on quirks mode or not) to either document.documentElement or document.body. Seems like less to have to change in the future, and less confusing for future web authors who never use quirks mode, and never developed during a time in which this bug existed, as to why there are 2 properties referencing document.documentElement.
If I have a simple boolean property, I can just replace /* whatever UA check they use now */; with document.behavesCorrectly || /* whatever UA check they use now */;
In closure, the change would be:
goog.dom.getDocumentScrollElement_ = function(doc) {
  // WebKit needs body.scrollLeft in both quirks mode and strict mode. We also
  // default to the documentElement if the document does not have a body (e.g.
  // a SVG document).
  if (!goog.userAgent.WEBKIT && goog.dom.isCss1CompatMode_(doc)) {
    return doc.documentElement;
  }
  return doc.body || doc.documentElement;
};

to:
goog.dom.getDocumentScrollElement_ = function(doc) {
  // WebKit needs body.scrollLeft in both quirks mode and strict mode. We also
  // default to the documentElement if the document does not have a body (e.g.
  // a SVG document).
  if ((!goog.userAgent.WEBKIT || document.behavesCorrectly) && goog.dom.isCss1CompatMode_(doc)) {
    return doc.documentElement;
  }
  return doc.body || doc.documentElement;
};

as opposed to:
goog.dom.getDocumentScrollElement_ = function(doc) {
  if ("scrollingElement" in document) {
    return document.scrollingElement;
  }
  // WebKit needs body.scrollLeft in both quirks mode and strict mode. We also
  // default to the documentElement if the document does not have a body (e.g.
  // a SVG document).
  if (!goog.userAgent.WEBKIT && goog.dom.isCss1CompatMode_(doc)) {
    return doc.documentElement;
  }
  return doc.body || doc.documentElement;
};

In the end, I commend @RByers for pushing this towards actually getting addressed, and I'm happy with whichever solution ends up happening here. Just wanted to make sure the case was made for alternate approaches.

So in the future, assuming the bug is successfully fixed, and you don't have to worry about current-era browsers, but still worry about quirks mode (like Closure does), with scrollingElement it would be like this:
goog.dom.getDocumentScrollElement_ = function(doc) {
    return doc.scrollingElement;
}

(or scrap the method altogether and use scrollingElement directly)
with behavesCorrectly it would be (assuming it returns false in quirks mode?)
goog.dom.getDocumentScrollElement_ = function(doc) {
    if (doc.behavesCorrectly) {
        return doc.documentElement;
    }
    return doc.body || doc.documentElement;
}

However this would not be technically correct (the current Closure script isn't, either), because document.body can be a frameset element, or the body can be independently scrollable (in which case, scrollTop should scroll the element, not the viewport, per spec), or if document.body is null it would give you the html element but that is not the "scrolling element" in quirks mode, and so on. Possibly being technically correct is a non-goal for Closure; it was a goal for the polyfill. With scrollingElement you get correct behavior "for free".

Shouldn't the scrollingElement property be part of the viewport instead of the document ?
Thus document.defaultView.scrollingElement or simply window.scrollingElement.

It's supposed to be a drop-in replacement for document.body or document.documentElement, which are on document.
Adding a property on the global is higher risk than adding a property on document.

@zcorpan thank you for the relevant point of view, I understand your reasoning behind both of them.
However some of the late additions to the global do not follow that exact reasoning.
For example API like window.requestAnimationFrame() and window.cancelAnimationFrame(), or speaking of recent properties additions window.localStorage and window.sessionStorage which are the new document.cookie alternatives/replacements.
For something older in time, see the window.location property which is a duplicate of the document.location property.
There are surely other examples of this.

@dperini - do you have an example of a property of window that refers to an element (not document)?
for (x in window) { try { if ("nodeType" in window[x]) console.log(x) } catch (e) {}}
Only outputs -
document
For me.

@phistuck,
no I don’t have such an example, but I don’t think that’s the way to measure the correctness of what is being implemented (wrongly in my humble opinion).
Other than that, my proposal was to have a getScrollingElement polyfill or a new method on the global space and not a new scrollingElement property on the document. There may already be enough aliases of the documentElement/body on the document node if we look at "DOM 1 Core" and the "DOM 2 Traversal".
This proposal does fit with your reasoning on the above code snippet and is future proof for when Webkit would fix the bug about Quirks/Standards modes behavior related to which element scrolls the viewport. That behavior have been in the specs for the last decade, the bug was spotted a long time ago but it was never fixed in Webkit. Now that bug was "forcedly" inherited also by Internet Explorer and Opera. It is the right time to fix it in the right way, instead of forcing everybody to make the wrong choice and the specs rewritten.
Have you noticed that everything "scrolling" related (properties and methods) are already bound to the viewport ? (see: scrollX, srcollY, scroll(), scrollBy(), scrollTo() and more).
Have you noticed that there is nothing "scrolling" related on the document node ?

@dperini - I am not sure mixing DOM concepts (DOM elements) and window (which is a global) is the right way to go but I defer to others for the rationale behind not adding such properties to window so far (I guess there is a rationale).
By the way, the methods you mentioned look like legacy methods (introduced by a single browser in the first browser wars and copied by others for compatibility or competitiveness) to me and not a result of consensus or deep thinking, really.
And, yes, I know they are specified as a part of the CSSOM View Module, but that does not mean a lot (it may merely document the APIs that are already prevalent and you cannot get rid of them, so they better be specified than otherwise).

@phistuck,
@phistuck,
just to say it clearer ... I am against adding a scrollingElement property to the document, it is already clear in the specs in which cases the scrolling element will be the body element (only in Quirks Mode), in all other cases the scrolling element should be the documentElement.
In any case, if that's deemed necessary to ease future detection I suggest adding a new method to the viewport (the global window).
Keep in mind that adding this new property or method doesn't solve anything for those that felt they should trust UA sniffing (the primary concern), they will be forced to change their code anyway if the bug is going to be fixed one day. The other way around is changing the specs and copy the bug also on Firefox and other already spec compliant engine. The latter, introducing far more changes in code written in the last decade.
My 2 cents :)

@phistuck,
just for fun, but following the reasoning in the above code snippet, you can see that on the document, excluding the Traversal properties, there is only one property pointing to an Element whose parentNode is a document node (nodeType == 9):
for (x in document) {
    try {
        if (!/Child$/.test(x) && document[x].nodeType == 1) {
            if (document[x].parentNode.nodeType == 9) console.log(x)
        }
    } catch (e) {}
}

which yields only one property:
documentElement

adding scrollingElement property to document will change the results to yield two properties:
documentElement
scrollingElement

Just noticed that in ChromeCanary, when enabling the "Experimental Web Platform Features" in a frameset document, the document.scrollingElement may be a reference to the frameset element.
Also, for a newly created document the same property value will be null. Not sure it is expected.

Also noticed that Safari is missing some of the "DOM 2 Traversal" properties/attributes on the document. Both Firefox and Chrome have firstElementChild and lastElementChild properties on the document node.

@dperini - why do you only look for nodes that have document as their parent node? It is a meaningless constraint (to me, anyway).
document.body, document.head, document.images[0] and so and so (yes, traversal as well) return nodes. Nothing on window does, except document which is the only entry point to the DOM.
Anyway, I am just stating the current state of affairs. It does not mean that there will never be relations from window to elements. If you still do not see my point or disagree, there is not much I can do to explain it and this discussion is pretty pointless, since I am certainly not an expert (but the TAG team is), so I defer to the experts.

That is expected.  document.body sometimes returns a frameset](http://www.w3.org/TR/html5/dom.html#the-body-element-0) too, and document.documentElement should be null in the empty document case.

@phistuck,
I didn't want to sound disrespectful, I am not an expert either, these were just my observations.
As there is only one property on window to drill-down to the DOM tree, there is only one property on document referring to an element having the parentNode to walk-up directly to the DOM tree host object, the document itself. But, I repeat, it was "just for fun" I am not trying to draw conclusions about those coincidences.

@RByers thank you for the answer, the clarification and the links to the relevant specifications.

@RByers

That is expected.

The frameset thing is a bug, the CSSOM View spec for scrollingElement does never return a frameset element. It specifically calls this out in a note. :-)
@dperini It seems to me that your line of reasoning is that the window is equivalent to "the viewport", but I think that is not the case. There is no object actually representing the viewport. So for some APIs it's the window, for some it's the document, for some it's the root element, for some it's the body element.

@zcorpan,

It seems to me that your line of reasoning is that the window is equivalent to "the viewport"

from what I understand this is almost exact, a more elaborate wording to describe that could be:
in a desktop browser, showing an non styled document, the window is equivalent to the document.defaultView which is the default document viewport ("porthole").
In mobile browsers having vendor supplied UA stylesheet, or in desktop browsers having user supplied stylesheet, the supplied style may affect and define the number of currently available viewports ("portholes"). This may be achieved through the overflow, width, and height CSS properties and their values.
To summarize ... there may be more than one viewport for an HTML document, though there is only one defaultView.

@zcorpan,
basically each DOM Element could be made to represent a viewport through the use of CSS overflow property combined with the width and height properties.

Hey all,
Thanks for the thoughtful discussion here.
Per most transitional periods for features, we might see pain for a long time as a result of any change. That means we aren't going to take a side about what Blink should do to manage a transition (if any).
The TAG's job is to make sure that when we arrive at a different world, it's the one that's actually better. A functional DOM that doesn't require wrapping ad infinitum is a goal we must aim for.
Lastly, multiple viewports aren't A Thing (TM) in the platform today. Many APIs we expose simply won't do the right thing in a multi-viewport world. They're the vestigial tail of a once hoped-for future. They do not weigh on the discussion at hand.
The current situation is bad/broken and that fixes which exacerbate the + issue won't fly. We also view the <iframe> solution as a non-starter. As a result, I'm going to write up a review that captures as much of the discussion here as seems reasonable with the punch-line being that the Blink proposal looks good.
The larger concern I've got is that none of this reads on the root issue: how did scrollingElement get decided? How can a programmer set it themselves? I.e., why is this magical? That'll be the meat of the discussion in the writeup.
Regards

@slightlyoff,
handling multiple viewports has never been the objective here, it was only a discussion follow-up.
I believe that https://github.com/dperini/scrollToAnchorRef project better explains my point on why I absolutely needed to determine the scrollingElement of the current defaultViewin a cross-browser way.

@zcorpan

The frameset thing is a bug, the CSSOM View spec for scrollingElement does never return a frameset element. It specifically calls this out in a note. :-)

Ah, thanks.  I see now that I recently introduced a bug here (where document.body.scrollTop would scroll the viewport even when body is a frameset).  I will get that fixed before it hits beta.

Point of information: this has recently been implemented in "Microsoft Edge" (Project Spartan), as per the agreement in: https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/scrollingelement/blink-dev/mJ5SlCj3CHc/PNSSYY5O_IQJ
It solves the problem of working around Blink's viewport bug (which Edge copied for interop), and also helps with the Quirks Mode use case. So, seems OK to me :-)

Discussed at Boston f2f.

@slightlyoff to write up some feedback.

We didn't get to a formal review doc on this, but provided feedback in comment here: https://github.com/w3ctag/spec-reviews/issues/51#issuecomment-98203476
Implementers converged on this behavior and the open item about setting .scrollingElement seems like something for the Houdini and CSS folks to wrestle with another day.
Closing, given that events converged on the TAG's preferred outcome.


[proposal] Have a dedicated Jupyter rainy day fund for gke.mybinder.org

Context
gke.mybinder.org is the largest member of the Binder federation. We have historically run it via credit donations from Google, and have scrambled to find credits from other stakeholders when these credits run out. Credits tend to come in 1-year batches with hard end-dates.
Because of the cyclical nature of credits, it creates a "credit crunch" where old credits may run out, but we have not yet found new credits for the infrastructure. This creates stressful situations where it's unclear how we'll pay for gke.mybinder.org. Most recently this happened in the issues below:

https://github.com/jupyterhub/mybinder.org-deploy/issues/2138
https://github.com/jupyterhub/team-compass/issues/430
https://github.com/jupyterhub/team-compass/issues/463

In these moments, individual stakeholders in the Binder project have stepped up to backstop gke.mybinder.org while we look for more credits, but this is a risky solution that depends on individual actors stepping up, and is a potential source of inequity amongst the Binder team members.
Instead, we should define a process that:

Reduces the risk associated with running out of credits on mybinder.org
Defines clear responsibilities for who must ensure that more credits are available to run the service

Proposal
As a first step, I propose that we set aside a dedicate account to backstop gke.mybinder.org. This account could be linked to the gke.mybinder.org Billing Account, so that whenever credits ran out, we would begin drawing from this account as a last resort. Our target would be to have at least 6 months of funding in the account at all times, to give us plenty of leeway if we need to find another round of credits or fundraise for it.
Note that most of the time, this funding would not be used - we still aim to power mybinder.org via credit allocations. This is just "gap funding" for when credits happen to run out.
How much cost are we talking about?
Historically, gke.mybinder.org costs around $7,000 per month (so, 6 months of funding would be roughly $42,000). However, we have recently undertaken several cost-saving measures, and believe that this is down to around $4,000 a month. So let's say $24,000 is a low-estimate for 6 months of usage. Ideally, we'd shoot for $50,000 in reserves if the funds were available, to give ourselves some breathing room.
Steps to implement this
I believe that we'd need to take the following steps:

[ ] Decide whether Jupyter wants to commit to this responsibility
[ ] Agree on a backstop amount, and find the source of this funding (potentially fundraising if necessary)
[ ] Define the charter for this funding, and the process around when we begin drawing down from this fund + how we will replenish it if we must draw down the funds.

cc @fperez @ellisonbg and @afshin who I think are the ones that recommended I open this issue / proposal. Please let me know if there's a different place that you'd like me to raise this issue.

This issue has been mentioned on Jupyter Community Forum. There might be relevant details there:
https://discourse.jupyter.org/t/governance-office-hours-meeting-minutes/1480/179

Sorry for the delay in responding,
I am in general +1 on the idea, I do have some question though.

50k is also a lot as i could cover a FTE for 6month. I'd like to see comparison on what those 50k could be used for. If we had 50k in an account I would be torn between having it sped on mybinder and someone to resurrect nbviewer.org that still have a huge amount of traffic.

I'd really like to see a mitigation plan in case we are short on funds to dramatically reduce mybinder spending if the case arise. Like a press-button solution that I don't know, halves the CPU allocated to each image, and implement a 30s wait period to start an image with a "Binder is out of funds" banner.

We've been discussing some of these things over on JupyterHub threads. One of the biggest issues with the current setup is that we have "n=1" things that are hard to turn off (federation-redirect, main DNS, central analytics, events archive) plus the GKE BinderHub deployment which is the vast majority of the cost, in the same place. One thing we can do is move the "this must run" stuff to a separate deployment, so that it's easier to just "turn GKE off", which rapidly takes most costs to zero (ideally, we'd have a federation that can handle it, but even when we don't all federation members can now be 'full' with come-back-later messages). That way, we could be paying for the inexpensive always-on stuff (we can work on cost estimates) with steady donation income, and rely on grants/other support for the GKE federation member cluster which may need to shut down once in a while.

I'd like to see comparison on what those 50k could be used for.

In the case of Binder, it would only be used to pay for cloud costs with Binder, if we ran out of other sources of these cloud costs. This would be untouchable funding that cannot be used for other things. So if we are using these credits, it should be a question of "do we want mybinder.org to keep running or not".
In a separate conversation, I think we should find ways to raise revenue that supports ongoing operations and development of these services, but this issue I am trying to scope specifically for emergency purposes (like the one we are in now).

I'd really like to see a mitigation plan in case we are short on funds to dramatically reduce mybinder spending if the case arise

I would also love to see a plan like this. But I do not have the bandwidth nor skills right now to work on it. Do you see it as a blocker for devoting central Jupyter funds to support mybinder.org?

I've tried to clarify this proposal by renaming this to be "Emergency fund" instead of "Rainy day" fund - I think the original title may have been misleading to make people think it was a "nice to have" kind of purpose. I think that "emergency fund" makes it clearer that this is just for emergency purposes.

So if we are using these funds, it should be a question of "do we want mybinder.org to keep running or not".

Sorry I was maybe unclear, this question was more "what could those funds be used for if they were not blocked for binder", it's literally assuming we have those 50k in the bank, is blocking them for binder preventing us from paying someone to do devops on nbviewer 8h per weeks for two years ?
As you point out later, we are all out of bandwidth, and if the choice is between paying cloud cost, and paying you to go raise some money, I most likely prefer the second one than the first one.
I don't have any opposition to Emergency/rainy day (I understood it the same). I want to better discuss the criticality of paying cloud cost vs people.

That way, we could be paying for the inexpensive always-on stuff (we can work on cost estimates) with steady donation income, and rely on grants/other support for the GKE federation member cluster which may need to shut down once in a while.

This is an interesting model given the current money constraints and probably fits better with @Carreau thoughts around the paying cloud cost vs people discussion (because you have more degrees of freedom to decide if you either want to support the GKE cluster or allocate that money for development instead).


[react-native] Mock findNodeHandle

Had to choose between the react-native repo or this, but chose this one because the issue is more about how to mock some code rather that react-native specific details.

I’m running into an issue where findNodeHandle is getting invoked, which results in:
/Users/eloy/Code/Artsy/emission/node_modules/react-native/Libraries/Renderer/src/renderers/native/ReactNativeStack.js:44
return nodeHandle.getHostNode();
                  ^

TypeError: nodeHandle.getHostNode is not a function
    at Object.findNodeHandle (/Users/eloy/Code/Artsy/emission/node_modules/react-native/Libraries/Renderer/src/renderers/native/ReactNativeStack.js:44:19)
    at Component.focus (/Users/eloy/Code/Artsy/emission/node_modules/react-native/Libraries/Renderer/src/renderers/native/NativeMethodsMixin.js:145:43)
    at Component.<anonymous> (/Users/eloy/Code/Artsy/emission/node_modules/react-timer-mixin/TimerMixin.js:18:16)
    at ontimeout (timers.js:386:14)
    at tryOnTimeout (timers.js:250:5)
    at Timer.listOnTimeout (timers.js:214:5)

I tried to modify react-native’s mock setup to:
  .mock('TextInput', () => {
    const TextInput = mockComponent('TextInput');
    TextInput.prototype.focus = function() {
      console.log("Focus, yo!");
    }
    return TextInput;
  })

Because the autoFocus prop of the TextInput component leads to focus being invoked, which in turn is the one that tries to use findNodeHandle.
I’m wondering if my override isn’t working because the method is actually defined in a mixin? And/or if you have other tips? Like, I’m not sure if findNodeHandle should ever really be used in tests, if not I’d like to PR a change to the mocks in the react-native repo.

As an alternative workaround to referencing the test object within the app, I added the following code to a setup file to ignore the autoFocus prop during mock creation:
jest.mock('TextInput', () => {
  const RealComponent = require.requireActual('TextInput');
  const React = require('React');
  class TextInput extends React.Component {
    render() {
      delete this.props.autoFocus;
      return React.createElement('TextInput', this.props, this.props.children);
    }
  }
  TextInput.propTypes = RealComponent.propTypes;
  return TextInput;
});

@jpklein Oh yeah, that’s definitely a better workaround than mine 👍 Alas both workarounds suffer from the prop no longer showing up in Jest snapshots :/

This is definitely not a Jest issue, it's how Jest is being used. This issue tracker is not the place for help requests such as this.

I guess I should have been more clear in that the setup for RN is incomplete. So this isn’t a help request for my specific application, it’s more a help request for how to fix the incomplete RN setup.

That is also not appropriate to be discussed on the Jest issue tracker but rather the RN one.

Fair enough 👍

@jpklein Your code doesn't work any more after upgrading React-Native form 0.45.1 to 0.46.1.
Any new workaround?

@xareelee I was unable to replicate your issue with RN 0.46.2. Please take further discussion to facebook/react-native#14434

@jpklein I found the issue was caused by getScrollableNode in VirtualizedList. I do the same mock on VirtualizedList to solve this problem temporally.
See https://github.com/facebook/react-native/issues/15000#issuecomment-315221218 for more info.

Hello, I'm still having this problem even with no autoFocus and autoCorrect in RN 0.48.4 and jest-expo 21.0.2,
thanks

I had to modify the workaround slightly for RN 0.56:
jest.mock('TextInput', () => {
  const RealComponent = require.requireActual('TextInput');
  const React = require('React');

  class TextInput extends React.Component {
    render() {
      return React.createElement('TextInput', {...this.props, autoFocus: false}, this.props.children);
    }
  }
  TextInput.propTypes = RealComponent.propTypes;
  return TextInput;
});


Save final images on S3 and call them into website

Currently images for scrolly website are being tracked through GitHub and stored to src/assets/
However, these will get saved to S3 and called into the website eventually.
These images can also then be removed and un-tracked from GitHub.

Images are no longer used, now the images are build through svg code. No longer an issue.


Test embedding functions

Description of changes
Since I've noticed that the 'chromadb.utils.embedding_functions' module has no unit tests, if I'm not mistaken... I took the initiative of adding them as I go along..

Improvements

Unittests

Documentation Changes
I don't think tests are affecting the docs
Feel free to point out any discrepancies if this does not follow the roadmap.

@Benakrab this is awesome! thanks for working on this. Let me know when its ready for review 👏

@jeffchuber at this point (95% coverage) I think it would be a good idea to review :)

@Benakrab awesome! you can take a look why tests are failing? looks like something wrong with not importing text2vec

@jeffchuber I think it would be useful if we keep track of openai, cohere, sentence_transformers, etc. in a separate requirements file. And may be, we can add them to be installed following the workflow..

@jeffchuber I added a new unit test for normalized embeddings. Let me know if anything is missing ? Thanks

@Benakrab thanks!! letting tests run now

@Benakrab Can you add this test to https://github.com/chroma-core/chroma/blob/main/.github/workflows/chroma-test.yml and https://github.com/chroma-core/chroma/blob/main/.github/workflows/chroma-integration-test.yml so it will run?

@jeffchuber Should I add also openai, cohere, sentence_transformers, etc. to requirements in order to be installed before tests run ?
Normally, tests associated with each library will be ignored if the library is not installed..

We don’t want to bloat our dependencies and prefer to instruct users to
install them if needed. Could we modify the test action to install the
needed set of dependencies?
On Mon, Jul 24, 2023 at 12:03 AM Benakrab @.***> wrote:

@jeffchuber https://github.com/jeffchuber Should I add also openai,
cohere, sentence_transformers, etc. to requirements in order to be
installed before tests run ?
Normally, tests associated with each library will be ignored if the
library is not installed..
—
Reply to this email directly, view it on GitHub
https://github.com/chroma-core/chroma/pull/703#issuecomment-1647329503,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABKW32IVFRJJ3BYM5AFBKOTXRYM3TANCNFSM6AAAAAAZIEZ6FA
.
You are receiving this because you are subscribed to this thread.Message
ID: @.***>

--
Hammad Bashir
EECS | Cal

@HammadB I added them to requirements_test.txt which can be used during action workflow. All the calls to those libraries are mocked, so tests run fast. Except for onnxruntime in which the tests follow the implementation (the model is light and it is already been installed).
I guess this should solve the issue and may be close the PR ;)
Tell me what you think ?

@Benakrab thank you for your continued work on this! we will get it landed next week :)

Hi @jeffchuber , Let me know if anything is missing ? Thanks

Hello @jeffchuber and @HammadB, is this PR still have relevance for chroma ? Please let me know if you have considered another method for testing embedding_functions ? Thanks

Hi @HammadB, can we give this PR a final review. I added a new job for embedding functions.. The tests successfully pass on my main branch workflow.. I guess this should close this PR.. Tell me what you think ?

Hello @jeffchuber, would you please tell me if his PR still relevant to chroma ? Because I can no longer maintain this PR for next months.


Does not implement [LivewireUI\Modal\Contracts\ModalComponent] interface.

I extended from ModalComponent but still I am getting an error "Exception
[App\Http\Livewire\EditPriceAlertException] does not implement [LivewireUI\Modal\Contracts\ModalComponent] interface."
Anybody can point me in the right direction to solve this?
<?php

namespace App\Http\Livewire;

use LivewireUI\Modal\ModalComponent;

class EditPriceAlertException extends ModalComponent
{
    public function render()
    {
        return view('livewire.edit-price-alert-exception');
    }
}

That's strange, this should work. Have you tried a fresh install of Laravel & Livewire to rule out any influence by your existing codebase or third-party packages by any chance? That would help to debug.


chore(deps): update dependency org.jadira.usertype:usertype.spi to v3.2.0.ga

This PR contains the following updates:

Package
Update
Change

org.jadira.usertype:usertype.spi
minor
3.1.0.GA -> 3.2.0.GA

Renovate configuration
:date: Schedule: At any time (no schedule defined).
:vertical_traffic_light: Automerge: Disabled by config. Please merge this manually once you are satisfied.
:recycle: Rebasing: Whenever PR is stale, or if you modify the PR title to begin with "rebase!".
:no_bell: Ignore: Close this PR and you won't be reminded about this update again.

[ ] If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

duplicate of https://github.com/Jasig/uPortal/pull/1559

Renovate Ignore Notification
As this PR has been closed unmerged, Renovate will now ignore this update (3.2.0.GA). You will still receive a PR once a newer version is released, so if you wish to permanently ignore this dependency, please add it to the ignoreDeps array of your renovate config.
If this PR was closed by mistake or you changed your mind, you can simply rename this PR and you will soon get a fresh replacement PR opened.


In bp function, WE should set ds:fill(1)  not set ds:zero()

I am  confused with the backpropagation process. In the bp function, when i equals params.seq_length, the ds shouldn't equal 1 but not 0?(your reset_ds()  set ds be 0) As we know, the outermost calculus should be 1 when calculus Composite function: dy = 1 * d(x^2)= 2x. using 0 will be dy=0 * d(x^2) = 0. Is this a bug?
  reset_ds()
  for i = params.seq_length, 1, -1 do
    state.pos = state.pos - 1
    local x = state.data[state.pos]
    local y = state.data[state.pos + 1]
    local s = model.s[i - 1]
    local derr = transfer_data(torch.ones(1))
    local tmp = model.rnns[i]:backward({x, y, s},
                                       {derr, model.ds})[3]
    g_replace_table(model.ds, tmp)
    cutorch.synchronize()
  end

Unrolled RNN optimizes loss function, which is independent of the last hidden state. Therefore, it's derivative with respect to this state is zero.


Endpoints 2.0.0b7 - Firebase, Service Account tokens

Spent a good few hours trying to get non-default Service Accounts and Firebase tokens to work with the endpoints framework. I wanted to feedback some issues, some work-arounds, and some general pointers for anyone else trying to do the same thing.
After following the quickstart guides on the endpoint docs I was able to get the tokens validating on the endpoint proxy. However, when the tokens reached the endpoints framework things started to go wrong -- the tokens failed to validate.
It seems a little odd that the framework re-validates the tokens, though I'm sure there is a reason why.
The endpoint proxy passes the X-Endpoint-API-UserInfo header, so perhaps we could just check that instead?
Regardless, after some tinkering I managed to get the endpoint framework to play nicely with Firebase tokens and custom Service Accounts. These are hacks, but they serve as a proof of concept.
Although it took a long time to figure out, there weren't actually that many modifications to make. All of the changes are in endpoints/user_id_token.py.
For reference, here is the api decorator I am using:
main.py
...

firebase_issuer = endpoints.Issuer(
    issuer='https://securetoken.google.com/[YOUR-PROJECT-ID]',
<EMAIL_ADDRESS>)

service_account_issuer = endpoints.Issuer(
<EMAIL_ADDRESS><EMAIL_ADDRESS>)

@endpoints.api(name='echo',
               version='v1',
               allowed_client_ids=[[SERVICE-ACCOUNT-CLIENT-ID], 'firebase_auth'],
               audiences={
                   'google_jwt': ['this.is.a.test'],
                   'firebase': ['[YOUR-PROJECT-ID]'],
               },
               issuers={
                   'google_jwt': service_account_issuer,
                   'firebase': firebase_issuer,
               })
...

This is basically the echo example from the quickstart, modified to support Firebase tokens and non-default Service Accounts.
General Comments:

The use of try-except with top-level Exceptions makes debugging incredibly frustrating because it swallows some important details about why the exception occurred. I can see why it is used in the code, but I think it could be handled better. As it stands you need a lot of debug logging throughout the code in order to work out why a token fails to validate.

The jwks_uri for each issuer needs to be the raw variant in order to work properly with the endpoints framework. However, the endpoints proxy only accepts the x509 variant. I therefore had to override the uris in the endpoint framework. This is not mentioned in the endpoint docs, as far as I can tell.

When using Firebase tokens, aud (audience) is your project ID. This doesn't appear to be mentioned in the endpoint docs.

Service accounts must supply their client_id as the azp field in order for the validation to pass.

_verify_parsed_token
Custom Security Issuers
By default the code checks _ISSUERS only. This does not account for custom security directives in the api e.g. Firebase. I modified the issuer check to include custom values from the api security definition.
...
def _verify_parsed_token(parsed_token, audiences, allowed_client_ids):
  # Verify the issuer.
  iss = parsed_token.get('iss')
  if iss not in _ISSUERS and iss not in<EMAIL_ADDRESS>'https://securetoken.google.com/[YOUR-PROJECT-ID]']:
    logging.warning('Issuer was not valid: %s', parsed_token.get('iss'))
    return False
...

Audiences
By default the code does not handle audiences being a dict (provider => audiences). I added a loop to iterate through each of the providers' audience values and check for a match. A potentially better solution could be to combine all of the provider audiences into a single set on init.
    ...
# Check audiences.
  aud = parsed_token.get('aud')
  logging.debug(u'AUD: {}'.format(aud))
  if not aud:
    logging.warning('No aud field in token')
    return False
  # Special handling if aud == cid.  This occurs with iOS and browsers.
  # As long as audience == client_id and cid is allowed, we need to accept
  # the audience for compatibility.
  cid = parsed_token.get('azp')

  valid_audience = False
  if aud == cid:
    valid_audience = True
  elif isinstance(audiences, dict):
    for provider, provider_audiences in audiences.iteritems():
      if aud in provider_audiences:
        valid_audience = True
        break
  elif aud in audiences:
    valid_audience = True

  if not valid_audience:
    logging.warning('Audience not allowed: %s', aud)
    return False
    ...

Firebase tokens
Firebase auth tokens do not include a cid value which results in a Client ID is not allowed message. To circumvent this, I added a check on see if iss is equal to https://securetoken.google.com/[YOUR-PROJECT-ID]. If it does then assign a cid of firebase_auth (see the main.py code above for the allowed_client_ids definition)
...
  if not cid and iss == 'https://securetoken.google.com/[YOUR-PROJECT-ID]':
    cid = 'firebase_auth'
...

_verify_signed_jwt_with_certs
By default the code only checks _DEFAULT_CERT_URI, which obviously causes Firebase, and Service Account, tokens to fail to validate. To remedy this, I modified the code to retrieve the aud (audience) value from token_body and used this to lookup the correct cert_uri for each token type.
  ...
  token_aud = parsed.get('aud')

  if token_aud == 'this.is.a.test':
    logging.debug(u'Service Account token found based on audience: {}'.format(token_aud))
    cert_uri =<EMAIL_ADDRESS>  elif token_aud == '[YOUR-PROJECT-ID]':
    logging.debug(u'Firebase token found based on audience: {}'.format(token_aud))
    cert_uri =<EMAIL_ADDRESS>  else:
    logging.debug(u'Could not determine token type based on audience: {}'.format(token_aud))
    cert_uri = _DEFAULT_CERT_URI
  ...

Complete Function Reference
endpoints/user_id_token.py
def _verify_parsed_token(parsed_token, audiences, allowed_client_ids):
  """Verify a parsed user ID token.

  Args:
    parsed_token: The parsed token information.
    audiences: The allowed audiences.
    allowed_client_ids: The allowed client IDs.

  Returns:
    True if the token is verified, False otherwise.
  """
  # Verify the issuer.
  iss = parsed_token.get('iss')
  if iss not in _ISSUERS and iss not in<EMAIL_ADDRESS>'https://securetoken.google.com/lh-endpoint-test']:
    logging.warning('Issuer was not valid: %s', parsed_token.get('iss'))
    return False

  # Check audiences.
  aud = parsed_token.get('aud')
  logging.debug(u'AUD: {}'.format(aud))
  if not aud:
    logging.warning('No aud field in token')
    return False
  # Special handling if aud == cid.  This occurs with iOS and browsers.
  # As long as audience == client_id and cid is allowed, we need to accept
  # the audience for compatibility.
  cid = parsed_token.get('azp')

  valid_audience = False
  if aud == cid:
    valid_audience = True
  elif isinstance(audiences, dict):
    for provider, provider_audiences in audiences.iteritems():
      if aud in provider_audiences:
        valid_audience = True
        break
  elif aud in audiences:
    valid_audience = True

  if not valid_audience:
    logging.warning('Audience not allowed: %s', aud)
    return False

  if not cid and iss == 'https://securetoken.google.com/lh-endpoint-test':
    cid = 'firebase_auth'

  # Check allowed client IDs.
  if list(allowed_client_ids) == SKIP_CLIENT_ID_CHECK:
    logging.warning('Client ID check can\'t be skipped for ID tokens.  '
                    'Id_token cannot be verified.')
    return False
  elif not cid or cid not in allowed_client_ids:
    logging.warning('Client ID is not allowed: %s', cid)
    return False

  if 'email' not in parsed_token:
    return False

  return True

def _verify_signed_jwt_with_certs(
    jwt, time_now, cache,
    cert_uri=_DEFAULT_CERT_URI):
  """Verify a JWT against public certs.

  See http://self-issued.info/docs/draft-jones-json-web-token.html.

  The PyCrypto library included with Google App Engine is severely limited and
  so you have to use it very carefully to verify JWT signatures. The first
  issue is that the library can't read X.509 files, so we make a call to a
  special URI that has the public cert in modulus/exponent form in JSON.

  The second issue is that the RSA.verify method doesn't work, at least for
  how the JWT tokens are signed, so we have to manually verify the signature
  of the JWT, which means hashing the signed part of the JWT and comparing
  that to the signature that's been encrypted with the public key.

  Args:
    jwt: string, A JWT.
    time_now: The current time, as a long (eg. long(time.time())).
    cache: Cache to use (eg. the memcache module).
    cert_uri: string, URI to get cert modulus and exponent in JSON format.

  Returns:
    dict, The deserialized JSON payload in the JWT.

  Raises:
    _AppIdentityError: if any checks are failed.
  """

  segments = jwt.split('.')

  if len(segments) != 3:
    # Note that anywhere we print the jwt or its json body, we need to use
    # %r instead of %s, so that non-printable characters are escaped safely.
    raise _AppIdentityError('Token is not an id_token (Wrong number of '
                            'segments)')
  signed = '%s.%s' % (segments[0], segments[1])

  signature = _urlsafe_b64decode(segments[2])

  # pycrypto only deals in integers, so we have to convert the string of bytes
  # into a long.
  lsignature = long(signature.encode('hex'), 16)

  # Verify expected header.
  header_body = _urlsafe_b64decode(segments[0])
  try:
    header = json.loads(header_body)
  except:
    raise _AppIdentityError("Can't parse header")
  if header.get('alg') != 'RS256':
    raise _AppIdentityError('Unexpected encryption algorithm: %r' %
                            header.get('alg'))

  # Parse token.
  json_body = _urlsafe_b64decode(segments[1])
  try:
    parsed = json.loads(json_body)
  except:
    raise _AppIdentityError("Can't parse token body")
  logging.debug(u'JWT Token Body: {}'.format(json_body))

  token_aud = parsed.get('aud')

  if token_aud == 'this.is.a.test':
    logging.debug(u'Service Account token found based on audience: {}'.format(token_aud))
    cert_uri =<EMAIL_ADDRESS>  elif token_aud == 'lh-endpoint-test':
    logging.debug(u'Firebase token found based on audience: {}'.format(token_aud))
    cert_uri =<EMAIL_ADDRESS>  else:
    logging.debug(u'Could not determine token type based on audience: {}'.format(token_aud))
    cert_uri = _DEFAULT_CERT_URI

  certs = _get_cached_certs(cert_uri, cache)
  if certs is None:
    raise _AppIdentityError(
        'Unable to retrieve certs needed to verify the signed JWT')
  logging.debug(u'Retrieved certs: {}'.format(certs))

  # Verify that we were able to load the Crypto libraries, before we try
  # to use them.
  if not _CRYPTO_LOADED:
    raise _AppIdentityError('Unable to load pycrypto library.  Can\'t verify '
                            'id_token signature.  See http://www.pycrypto.org '
                            'for more information on pycrypto.')

  # SHA256 hash of the already 'signed' segment from the JWT. Since a SHA256
  # hash, will always have length 64.
  local_hash = SHA256.new(signed).hexdigest()

  # Check signature.
  verified = False
  for keyvalue in certs['keyvalues']:
    try:
      modulus = _b64_to_long(keyvalue['modulus'])
      exponent = _b64_to_long(keyvalue['exponent'])
      key = RSA.construct((modulus, exponent))

      # Encrypt, and convert to a hex string.
      hexsig = '%064x' % key.encrypt(lsignature, '')[0]
      # Make sure we have only last 64 base64 chars
      hexsig = hexsig[-64:]

      # Check the signature on 'signed' by encrypting 'signature' with the
      # public key and confirming the result matches the SHA256 hash of
      # 'signed'.
      verified = (hexsig == local_hash)
      if verified:
        break
    except Exception, e:  # pylint: disable=broad-except
      # Log the exception for debugging purpose.
      logging.debug(
          'Signature verification error: %s; continuing with the next cert.', e)
      continue
  if not verified:
    raise _AppIdentityError('Invalid token signature')

  # Check creation timestamp.
  iat = parsed.get('iat')
  if iat is None:
    raise _AppIdentityError('No iat field in token')
  earliest = iat - _CLOCK_SKEW_SECS

  # Check expiration timestamp.
  exp = parsed.get('exp')
  if exp is None:
    raise _AppIdentityError('No exp field in token')
  if exp >= time_now + _MAX_TOKEN_LIFETIME_SECS:
    raise _AppIdentityError('exp field too far in future')
  latest = exp + _CLOCK_SKEW_SECS

  if time_now < earliest:
    raise _AppIdentityError('Token used too early, %d < %d' %
                            (time_now, earliest))
  if time_now > latest:
    raise _AppIdentityError('Token used too late, %d > %d' %
                            (time_now, latest))

  return parsed

Hi I was trying to do the same thing for days now trying , to get endpoints and firebase authentication  working together, is your workaround still the best way to do it ? or did google fix this and make it less confusing ?

Hey @Tungamirai. I haven't checked out the latest version but, it doesn't look like the users_id_token.py file has been modified in the repository. I'm going to assume this is still a problem.

@lhmatt  thanks for the reply , are they any solid examples/tutorials form start to end that use endpoints , with firebase authentication or any full repositories or complete projects  that show this working ?
I  just started to learn app-engine and endpoints and it just seems very frustrating in general, was expecting it to be better documented with solid examples,   are problems like this common with endpoints ? , I dont understand why google hasn't just made a solid example that actually works

No worries, @Tungamirai. I have not found a full example with firebase auth, hence my post above. The closest I found was in the docs: https://cloud.google.com/endpoints/docs/frameworks/python/authenticating-users but I couldn't get it to work during the beta. Definitely give it a go if you can.
This version of endpoints has just come out of beta so it's understandable that the docs may be missing some information. The best thing you can do is submit feedback (there is a button the the bottom of each page on the docs) to help them improve the information available.
Failing that, there is almost always someone on StackOverflow that has experienced the same problems as you!

Thanks @lhmatt  , will be keeping my eyes open for a documentation update , the link you found is the same one i also found but also couldn't get it to work, I hope the docs are updated soon
Thanks again

@lhmatt Thanks so much for your post. It allowed me to finally get Firebase authentication working.
Google devs.....any word on when we can see the issues with Firebase authentication repaired in endpoints?

Thank you so much! I've been banging my head against the wall for days trying to get this working and just now found this post. I ended up using google.auth.id_token.verify_firebase_token() to verify the token on one service and spawning a new microservice running pyrebase just to get user information. I really hope this gets fixed quickly.

@lhmatt Thank you so much for this. I spent a good part of my day trying to get my app to work, looking through the endpoints source code and thinking that theres no way this Firebase thing should work as described in the guides...
Are you planning on making a pull request for this at all? If not, I'll try to make a fix based on your PoC.

No worries, @sxhan. I've abandoned my efforts with this framework so feel free to create your own pull request.
After quite a bit of investigation, I've moved on to using Endpoints via ESP on App Engine flexible. I know not everyone can/wants to move to the flexible environment but it works for what I need to do. You trade higher running costs for developer productivity - with GAE flexible you can use the existing OpenAPI (swagger) tools to generate server stubs and client libraries, and you're not reliant on Google to build features into this framework.
Note also that you can use both standard and flexible environments in the same project.

The example given for 3rd party issuer is like the following:
@endpoints.api(
        audiences={'auth0': ['aud-1.auth0.com', 'aud-2.auth0.com']},
        issuers={'auth0': endpoints.Issuer('https://test.auth0.com',
                                           'https://test.auth0.com/.well-known/jwks.json')})

And version 2.0.5 works with the custom issuer I am using.

@lhmatt Thanks for the suggestion! I was ready to do that yesterday before I found your post, but your reply is making me consider going that route again. Will probably play with this framework a bit more.
@Al77056, thanks for the note. Unfortunately I can't get my Firebase Authentication to work without the changes lhmatt mentioned despite being on 2.0.5 . Are you testing your app locally or in the cloud? I see the following comment from a recent commit:
+  If `endpoints_management.control.wsgi.AuthenticationMiddleware` is enabled,
 +  this returns the user info decoded by the middleware. Otherwise, if the
 +  current request uses an id_token, this validates and parses the token against
 +  the info in the current request handler and returns the user.  Or, for an
 +  Oauth token, this call validates the token against the tokeninfo endpoint and
 +  oauth.get_current_user with the scopes provided in the method's decorator.

Which looks promising. Looks like the AuthenticationMiddleware is supposed to be doing the token verifications and overriding the _ENDPOINTS_USER_INFO environment variable which skips all the verification logic in users_id_token.get_current_user(). The question is... how do I enable that?

@sxhan, I am testing in the cloud. Before changes made in 2.0.5, Endpoints framework was not validating JWT signed by Google service account. 2.0.5 fixes that.

@sxhan Could you provide sample logs of what happens when you try to use Firebase auth? 2.0.5 should indeed fix it.

@tangiel probably this screeshot of logs can clarifay.
I'm using GAE standard, the GAE users API for internal use and firebase auth to authenticate users requests.
As you can see in the screenshot, endpoints.get_current_user() returns the GAE users obj (line 1), the same obj of google.appengine.api.users.get_current_user().
But reading the docs, it should returns the google.api.auth.user_info obj (line 2) which contains the firebase user_id (line 3).

Ah, so the issue isn't really the type of the user object returned, rather that it doesn't contain the id. Correct?

I don't want to create confusion, I just start from the snippet example in the documentation https://cloud.google.com/endpoints/docs/frameworks/python/authenticating-users#authenticating_with_firebase_auth
where user = endpoints.get_current_user() return the something related with firebase and not with GAE users API.

Looks like I'm hijacking the thread a bit. Apologies in advance for that.
@tangiel, my problem is different than that of @presveva. My issue was that I kept getting ERROR    2017-05-16 20:26:38,633 users_id_token.py:362] Token info endpoint returned status 400: Invalid Value whenever I tried to make a request to the endpoint.
I now realize that it may be because I was testing my endpoints application in the local development server (via dev_appserver.py). I tried deploying to the cloud roughly following the instructions here and then here and was able to get token validation working and the user's email address returned via endpoints.get_current_user().
However, I am unable to get it working with the local server. I see that the code path for local server is a little different than for when its deployed, so I'm not sure whether I am doing something wrong or if its simply not supported. I did notice that local development server usage was not mentioned anywhere in the docs, so might simply be trying to use a non existent feature. Your feedback is appreciated!
Following the example Al77056 provided, this is how I'm setting up the endpoints decorator:
firebase_issuer = endpoints.Issuer(
    issuer='https://securetoken.google.com/tutorial-165519',
<EMAIL_ADDRESS>@endpoints.api(name='echo', version='v1',
               issuers={"firebase": firebase_issuer},
               audiences={"firebase": [PROJECT_ID]},
               allowed_client_ids=["firebase_auth"])

@sxhan Did you make it work by appling the changes in 'endpoints/user_id_token.py' as decribed by Ihmatt or was that no longer neccessary for you?

@sllegendre, following lhmatt's suggestions made it work, but I still have trouble without implementing those changes. I ended up writing a wrapper around endpoints.get_current_user() for my app to make development easier for myself. Something like this...
def get_current_user():
    """Returns the endpoint authenticated user object"""
    class FakeUser:
        def email(self):
            return<EMAIL_ADDRESS>
    if (os.getenv('SERVER_SOFTWARE', '').startswith('Google App Engine/') or
        os.getenv('SERVER_SOFTWARE', '').startswith('Development/1.0 (testbed)')):
        user = endpoints.get_current_user()
        if not user:
            raise endpoints.UnauthorizedException('Authorization required')
    else:
        user = FakeUser()
    return user

This made it easy for me to test all my endpoints locally, although it won't help you debug any configuration issues with firebase auth.
Note: the call to os.getenv('SERVER_SOFTWARE', '').startswith('Google App Engine/') to check for development vs. production environment is from the GAE docs, so that should be safe to do.

@sxhan I was wondering how to abstract this for use in unit tests. There's also the users.get_current_user method, so the FakeUser may not be required.

@lhmatt thanks ,my project working with firebase and endpoint.but my issue is when I add file from Google drive with help of picker library of JavaScript then I try to store these document .that time I get 401(not authenticate ) error


MONDO:0002799 & MONDO:0016710 [Merge]

Mondo term (ID and Label)
MONDO:0002799 nodular medulloblastoma
Reason for deprecation
Based on overlapping x-refs and some shared synonyms, I think they may be the same clinical presentation.
Term to be merged with
MONDO:0016710 medulloblastoma with extensive nodularity
Your nano-attribution (ORCID)
If you don't have an ORCID, you can sign up for one here

After reviewing the xrefs, I agree these should be merged. The two terms share the xrefs (NCIT:C5407 and UMLS:C1334970) and the other xref on 'nodular medulloblastoma' MONDO:0002799 of DOID:3873 has a synonym that matches the preferred name of 'medulloblastoma with extensive nodularity' MONDO:0016710.


GH-36241: [Packaging] Drop support for Amazon Linux 2

Rationale for this change
We want to require CMake 3.16 or later. (GH-34921)
If we drop support for Amazon Linux 2, we can require CMake 3.16 or later.
What changes are included in this PR?
Remove Amazon Linux 2 related codes.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.

@github-actions crossbow submit -g linux

+1
No objection on the mailing thread: https://lists.apache.org/thread/1gfpg0pxhk10n10lo0snx68t3pxfs2sl


Browser "back" button

Ionic version:  (check one with "x")
[x ] 3.x
I'm submitting a ...  (check one with "x")
[ x] bug report
[ x] feature request
Current behavior:
I'm trying to create progressive web application, not mobile application, that will work in desktop and mobile browsers.
I have nested ion-nav directives (root component has ion-nav in template and it loads second component with ion-nav in template)
import {SecondComponent} from 'start'

@Component({
  template: '<ion-nav [root]="rootPage"></ion-nav>'
})
class RootComponent {
  rootPage = SecondComponent;
}

import {ThirdComponent} from 'start'

@Component({
  template: '<ion-nav #myNav [root]="rootPage"></ion-nav>'
})
class SecondComponent{
  rootPage = ThirdComponent;
  @ViewChild('myNav') nav
}

I defined DeepLinker configuration for all components. So when application starts I have this path in browser address bar http://localhost:8100/#/second/third.
Then I run nav.setRoot(FourthComponent) in second component and I got this path in browser address bar http://localhost:8100/#/second/fourth.
Then I press back browser button and I got this path in browser address bar http://localhost:8100/#/second/third but in ion-nav of root component I get rendered two SecondComponent-s (one is hidden).
Expected behavior:
After back button pressed I expect to have this structure in html:
RootComponent -> ion-nav -> SecondComponent -> ion-nav -> ThirdComponent
Instead I get:
RootComponent -> ion-nav -> [(SecondComponent -> ion-nav -> FourthComponent : hidden), (SecondComponent -> ion-nav -> ThirdComponent)]

Hello, thanks for using Ionic! So there are a few things going on here. First, we do not recommend or support nesting ion-nav's at the moment and because of this I am going to close this issue. Second, and this not really specific to the issue your facing which is more coming from the fact that you have nested ion-nav's, but we are currently refactoring ion-nav to better handle url's, lazy loading etc so that it works better for PWA's.

@jgw96 So are you planning to support nested ion-navs in the future.
I found that current implementation of NavController has this method getActiveChildNav()


Use guice for better modularization

involve guice for better modularization.

@JiamingMai Please help add a list of items if any for further work on guice modularization, e.g. test improvement or others

@JiamingMai Please help add a list of items if any for further work on guice modularization, e.g. test improvement or others

TODO List:

Tests improvement with guice's dependency injection.
Modularization of WebServer and DomainSocketServer in Worker.
Modularization of other components.
Clean up the codes to remove all the BlockStore related codes.

@huanghua78 @dbw9580 Please leave additional comments or reviews if any and @JiamingMai please help address with future PRs

alluxio-bot, merge this please


[Do not merge] Attempt to remove _Strideable

Or feel free to merge it if you can get it to compile and test!
Demonstrates odd overloading behavior. _Strideable should not be necessary, it is there to ensure CountableRange is created by ..< when possible. Since Strideable: Comparable, this ought to happen but doesn't. That or I've done something wrong and someone can spot the quick fix...

@swift-ci please smoke test

@xedin got a bit further on this, tracked it down partly to how contains was implemented, which could be worked around. But I think there were some other blockers that were less easily revolved.

Well, it's currently failing with:
error: operator '..<' declared in type 'Comparable' must be 'static'

Closing as this is superseded by https://github.com/apple/swift/pull/12520


[Cool feature] Sort layers based on the position of the top left corner of the layer

Sorting would from top to bottom and left to right. Thoughts on how useful this is?
This is a feature that can come later.

@Mxchaeltrxn Yes! This is something I’m always wanting to do in other design tool. 😀


[Nomination] I would like to nominate Highlander Nodes

Who are you nominating?

Highlander Nodes (@Highlander-maker) that has picked up Rust recently and has worked with the @OmniFlixNetwork team to produce videos that will be shared with the community soon.

What are you nominating them for?

To work on HighlanderNodes' own idea of building a Telegram bot to interface with the Juno Network.

Please provide links to their work.
Below is the code for the bot, currently work in progress for the Juno Network

URL - https://github.com/Highlander-maker/juno-tg-bot/tree/cosmJS

i want to nominate @giansalex for his contribution of the code. he made it work :)

thanks @svabhishek :)..

Awesome! Do add links for the videos when they are done.

💯

Awesome @Highlander-maker.

👍

great work and a great community member that is always willing to help out with anything!

@blockcreators thank you mate. Its been a pleasure and i aim to continue on with Juno for years to come :)

@Highlander-maker from #27:

Contribution to 4 projects which are still being developed further so i will update as i go along. Im not a prolific coder. Im not looking to blag Juno. i just want to highlight the time and effort to learn and hopefully contribute to this amazing community and highlight the amazing work put in by @giansalex .. Without him none of 3 of these project would even be on this list so thank you. Thank you to @JakeHartnell @the-frey @nullmames @jackzampolin @dimiandre for their help. Also, with out a doubt @Wolfcontract @blockcreators and not forgetting @toschdev for showing me the door to Cosmos eco-system. I am now have a basic toolset and knowledge to go forward and perform in future developments now. :). So thank you very much.

Here is what ive been working on to date.

Juno Telegram Bot
This bot has the ability to upload a compiled byte code on Juno chain. (Needs server to run on but npm you can run it locally). > Gianny helped with linking this code to work on Juno.
https://github.com/Highlander-maker/juno-tg-bot

Cosmwasm IDE ( writing suggestions for IDE and the use on Juno network as a ETH user on Remix)
Helped Orkin with a script on what to have in the IDE with my remix experience. As a user of this in the past coding projects on > ETH Orkin was after more of what executional functions were required for deploying contracts on chain. (JUNO)
InterWasm/DAO#6

Work on Cosmverse (in progress) @giansalex big input on this. (STILL DEVELOPING)
https://github.com/CosmosContracts/cosmverse

CoinFlip Dapp
Simple betting app game linked on Juno network. Bet 50/50 your Juno for the chance to double up. @giansalex and myself
wrote the code. i did it in Solidity while he re wrote in to Rust with the use of CW20
https://github.com/Highlander-maker/CoinFlipDapp

On #26, @orkunkl wrote:

He helped me to write a spec for CosmWasm IDE based on his Remix experience

Mentioned on #21

copied from https://github.com/CosmosContracts/hack-juno/issues/64
Who are you nominating?
Highlander @HighlanderCosmos
What are you nominating them for?
Many months of work on Cosmverse in collaboration with @giansalex
Please provide links to their work.
https://cosmverse.com


DynamoDBV2 memory leak .NET Core 2.1

We are facing a memory leak issue while getting response back from the server. Every request increases memory from 500 to 100 MB (tests were taken for 5.25 MB of data).
Expected Behavior
RAM should be released after response.
Current Behavior
For example, we send a request that should return us 5.25 MB of data, but RAM increases about 500 MB when we get a response and remains the same, so that the next same request increases RAM up to 1GB, then with another same request RAM increases up to 1.5GB etc. This issue appears on both low-level and high-level implementation approaches.

Before request(83 MB of RAM)

After request (550 MB of RAM)

Postman response

Possible Solution
There is no possible solution.
Steps to Reproduce (for bugs)
Send multiple requests with large amount of response data.
Context

Our application is hosted in ECS container with 2GB of RAM, so when a lot of requests being send to the server, at some point of time container will be stopped and restarted because of memory overflowing.

Your Environment

AWSSDK.Core version used: <IP_ADDRESS>
Service assembly and version used:
Operating System and version: Windows 10 Pro, version 1803,
Visual Studio version: VS 2017 Version 15.9.5
Targeted .NET platform: 4.8.03761

.NET Core Info

.NET Core version used for development: 2.1
.NET Core version installed in the environment where application runs: 2.1

What kind of request are you sending?  If you can provide a code sample, that may be helpful.

We are sending a query request. Here is our code snippet:
List<CarShop> response = null;
            try
            {
                response = context.QueryAsync<CarShop >(
                request.Environment,
                QueryOperator.Between,
                values.Cast<object>(),
                config).GetRemainingAsync().Result;
            }
            catch (Exception ex)
            {
                logger.LogError($"Error while trying to get a response from Dynamo Db\nMessage: {ex.Message}\n{ex.StackTrace}");
                throw;
            }

Context is a IDynamoDBContext instance that is injected to the class.
General amazon sdk related configuration in our startup class:
services.AddDefaultAWSOptions(this.Configuration.GetAWSOptions());
services.AddAWSService<IAmazonDynamoDB>();
services.AddScoped<IDynamoDBContext, DynamoDBContext>();

And last but not least the model of our table:
 [DynamoDBTable("cars-table")]
    public class CarShop : IFeatureModel
    {
        public CarShop ()
        {
            Id = Guid.NewGuid();
            CarOwner= new List<CarOwner>();
        }

        [DynamoDBRangeKey]
        public Guid Id { get; set; }

        [DynamoDBHashKey(AttributeName = "E")]
        public string Environment { get; set; }

        [DynamoDBLocalSecondaryIndexRangeKey("CollectTimeIndex")]
        [DynamoDBProperty(AttributeName = "CT", Converter = typeof(DateTimeConverter))]
        public DateTime CollectTime { get; set; }

        [DynamoDBProperty(AttributeName = "AN")]
        public string AccountName { get; set; }

        [DynamoDBProperty(AttributeName = "AT")]
        public AccountTypes AccountType { get; set; }

        [DynamoDBProperty(AttributeName = "ANum")]
        public string AccountNumber { get; set; }

        [DynamoDBProperty(AttributeName = "C")]
        public string Country { get; set; }

        [DynamoDBProperty(AttributeName = "SD")]
        public List<CarOwner> CarOwner { get; set; }
    }

public class CarOwner
{
        [DynamoDBProperty(AttributeName = "S")]
        public string Site { get; set; }

        [DynamoDBProperty(AttributeName = "CM")]
        public CarModel CarModel{ get; set; }

        [DynamoDBProperty(AttributeName = "CN")]
        public string CarNumber { get; set; }

        [DynamoDBProperty(AttributeName = "C")]
        public int Count { get; set; }
}

public interface IFeatureModel
{
        DateTime CollectTime { get; set; }
}

*Some attributes and classes were renamed due to security purposes.
Few notes that need to be mentioned:

We have tried both scan and query requests to the table.
We have performed operations to multiple table just to test the difference. The problem with memory remains the same.
We have even created a default project and added just one simple request  to the table and the problem haven't gone
We have tried to register context as Transient, Scoped and Singleton. Nothing has changed

We've had some other concerns about memory leaks in DynamoDB.  We're doing some investigation on it this week, but we can't make any promises on a resolution time at the moment.
I have a few other questions for you:

The GetRemainingAsync method will make many smaller calls to get the next set of data.  I'm assuming that one of these small calls is the 5.25 MB that you are seeing.

What is the purpose of casting the values to object?

Have you looked at other alternatives for doing the calls asynchronously?  As is, your code will block the calling thread until all results are ready.

So, we have tried a lot of approaches for making the request to the table  including the asynchronous one:
var response = await context.QueryAsync<TransmissionsReport>(
request.Environment,
QueryOperator.Between,
values.Cast<object>(),
config).GetRemainingAsync();

It doesn't help us. The problem remains the same.
The values object is DateTime list:
var values = new List<DateTime>()
{
    // items
}

But we've also tried to make an asynchronous request without values object, just specifying a hash key:
var response = await context.QueryAsync<TransmissionsReport>(
                 request.Environment).GetRemainingAsync();

Unfortunately, it hasn't affected anything.
The table's data has increased since issue was published so the response. But we can test the correctness of the response data by checking list's count of response object and compare it to the expected amount of items. In all of the cases GetRemainingAsync works properly.

Hey guys,
When you call GetRemainingAsync, the results are being collected in the Search document model.
        /// <summary>         /// Whether to collect GetNextSet and GetRemaining results in Matches property.         /// Default is true. If set to false, Matches will always be empty.         /// </summary>         public bool CollectResults { get; internal set; }
Can that be set via a parameter of GetRemainingAsync or on the response object of QueryAsync?


[Policy Assistant] display enhancements from KubeCon demo (PR 1/2)

A few enhancements:

display (b)anp rule names in the table and "Flow"
make connectivity table easier to read for loopback traffic
stop adding an extra row in the explain table when there are no egress policy rules

New flow example:
[NPv1] Allow (rule-name)

Loopback traffic example (adding "#")
simulated connectivity:
+--------+--------+--------+
| TCP/80 | DEMO/A | DEMO/B |
| TCP/81 |        |        |
+--------+--------+--------+
| demo/a | # X    | . X    |
+--------+--------+--------+
| demo/b | . X    | # X    |
+--------+--------+--------+

I like this -- making those undefined cases explicit!
/lgtm


can you kindly create a module for NR?

I would like to avoid use mqtt and running your script externally, may you please create a module with this code? Thank you a lot!!

Hello thanks for your interest. The library should be useable as a module but will check that out.  Converting to MQTT is one of the main reasons for it - so it's not something I would really want to remove;
When you say NR?  could clarify what is please/

Thank you for your kind answer! :)
I meant Node Red, having your koda available there as a module can simplify a lot for users that don't need MQTT for their projects. Also, having one process running (Node Red only) does simplify the management in case of crash (restarting, etc).
I hope it can be easy to port it there, thank you a lot!

The onvif module available now on NR is only able to ask a screenshot, and I'm aim to get on NR notifications of movements and alarms, I think it can be really useful to many more users

Ah Node-Red... got you! Know it very well;
In my case the MQTT feed is being handled by NodeRED to take actions. will give it some thought!

please consider my help as a tester with different camera's technologies :)


[BUG] group::Tabs contents not visible

Describe the bug
The group::Tabs tabs are too tall and the content is not visible with fltk-rs-1.4.20
To Reproduce
Run the closable_tab example with fltk-rs-1.4.20
cargo run --example=closable_tab
Screenshots

Desktop info

OS: ArchLinux (rolling release)
Version: fltk-rs-1.4.20
Window Manager: KDE Plasma 5.27.10, wayland

Additional info
The tabs example shows the widget contents, however if you increase the height of the window, the height of the tabs increase.  In this cropped screenshot the tabs example window height is >1500px.

Hello
Thanks for the report.
I was able to reproduce the issue, and have applied a fix in the overriden resize() method, which affected Tab::auto_layout().
Can you try with the current master?
Thank you

Thanks for the fix.
The tabs in my app work correctly with fltk-rs commit 6f5b261d643f265b806cbfdc383ac787233e8135


the text order is reversed when using highlightElement().  with contenteditable attribute.

Describe the issue/behavior that seems buggy

❌ when typing it adds the text/code in reverse when typing.

Sample Code or Instructions to Reproduce

when using contendeditable attribute in the  HTML tag element.

with an input event listener
const code = document.querySelector("code");

code.addEventListener("input", () => {
    hljs.highlightElement(code);
    // other code logic (mostly saving to localStorage)
}

like you saw in every typed letter I will add the highlight functionality,
 but seems that completely breaks the typing direction

and yes, input events also work for not <input> tags if they have that attribute (contenteditable)
 docs: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/input_event

source code:
https://jsfiddle.net/20zpL5bk/

also another bug is that we can't go down the line break. (only if there is hljs.highlightElement(code);)

✅ here without hljs.highlightElement(code); is super fluid and can't do everything (but not syntax highlight):

(how can I have fluid, normal writing with also syntax highlighting in real-time (input event)?
 is my code wrong for some reason?)

Expected behavior

✅ how should be:

 or hello world

❌ how it is now: >vid/<>vid< or dlrow olleh

Additional context

I tried to write the code in reverse myself manually >vid/<dlrow olleh>vid<,  and yes the highlighting works.

but the direction is always the wrong way. (also deleting isn't possible)

another interesting thing is if you copy and paste (CTRL+V) the correct code
hello world
 it works fine. (but still remain in the same line, no multiple lines)

also the cursor of the input doesn't move with the text but remains at the start

source code:
https://jsfiddle.net/20zpL5bk/

solved one problem of the 2 ones:

the reversed text input.

this is happen because every time that input event fires,
it will reset the contenteditable input.
by resetting, I mean that the caret position is set to 0.
so technically, every time a letter is pressed it is added at the start and not at the end.
like you see here:

so the solution is to move the cursor to the end.
this is the solution: https://stackoverflow.com/a/69727327/17716837
function setEndOfContenteditable(code) {
    let sel = window.getSelection()
    sel.selectAllChildren(code)
    sel.collapseToEnd()
}

and adding this functionality to the end of the input event
code.addEventListener("input", () => {
    hljs.highlightElement(code);
    setEndOfContenteditable(code);
});

however, still wondering why this bug happens with the hljs method and without it not happen.

Guess: Swapping out the content is resetting the cursor (to the beginning)... so you'd need to save and restore the cursor position "properly" before/after calling highlightElement.  Just a guess.
This isn't something we officially support though - it usually leads to all sorts of glitches, like the one you're seeing - but I'll leave this open a a little while to see if anyone else wants to jump in and help you.

@joshgoebel thanks


fix MISRA-C rule 20.9 (#ifdef instead of #if)

MISRA-C states that If an attempt is made to use a macro identifier in a preprocessor directive, and that identifier has not been defined, then the preprocessor will assume that it has a value of zero and this could not be what was expected.

@andrewboie is just a style problem ? I can change them to use ifdef but makes sense keep the second comparison (CONFIG_ != 0).

Absolutely we don't want this for kconfig variables.  Those have a standard form and are always testable with #ifdef by design.
Some of the other stuff is more squishy.  I'm pretty sure all those xtensa HAL symbols, in fact, are always defined to either 0 or 1 specifically.  But even then it sounds like MISRA would be fine with that as  there's no ambiguity about what to do for an undefined symbol, right?

Absolutely we don't want this for kconfig variables. Those have a standard form and are always testable with #ifdef by design.

agree.
This looks really bad for something we use in mass.

Absolutely we don't want this for kconfig variables. Those have a standard form and are always testable with #ifdef by design.

agree.
This looks really bad for something we use in mass.

the problem is if we do #if X and X is not defined violates a MISRA-C rule. Changing #if to #ifdef changes the semantic, e.g:
#define CONFIG_A 0

#if CONFIG_A
/* code no compiled */
#endif

#ifdef CONFIG_A
/* code compiled */
#endif

So, what I'm proposing is:
#ifdef CONFIG_A && CONFIG_A != 0

Yeah, but the point is that kconfig also has defined semantics, and it doesn't do that.  A false kconfig value produces an undefined symbol.  That's it's documented API.  And hell, we even control the code generator.

I guess to state things differently: where does this end?  I mean, a naive interpretation of this would would seem to require that every preprocessor variable be first checked with defined() before it is used.  Which is... insane, right?  And even MISRA doesn't require that, as I'm understanding the rule.  It just wants us to assume "undefined means zero".  Which... is exactly what almost all the usage being changed here was doing in the first place.

I guess to state things differently: where does this end? I mean, a naive interpretation of this would would seem to require that every preprocessor variable be first checked with defined() before it is used. Which is... insane, right? And even MISRA doesn't require that, as I'm understanding the rule. It just wants us to assume "undefined means zero". Which... is exactly what almost all the usage being changed here was doing in the first place.

you are right, i just added the second test to keep the same semantic in the code (with the if).
I don't have problem changing it to use ifdef, this is completely safe for the Kconfig ones, but there others that may need special treatment. As I did it automatically I was conservative with the change.

Codecov Report

Merging #12341 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #12341   +/-   ##
=======================================
  Coverage   48.32%   48.32%
=======================================
  Files         294      294
  Lines       44189    44189
  Branches    10600    10600
=======================================
  Hits        21354    21354
  Misses      18569    18569
  Partials     4266     4266

Impacted Files
Coverage Δ

kernel/include/kernel_structs.h
100% <ø> (ø)
:arrow_up:

include/net/net_pkt.h
85.14% <ø> (ø)
:arrow_up:

kernel/sched.c
92.83% <ø> (ø)
:arrow_up:

kernel/init.c
72.13% <ø> (ø)
:arrow_up:

include/logging/log_core.h
100% <ø> (ø)
:arrow_up:

include/misc/util.h
50% <ø> (ø)
:arrow_up:

arch/posix/core/posix_core.c
91.91% <ø> (ø)
:arrow_up:

include/shell/shell_history.h
0% <ø> (ø)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 330cbfa...095f787. Read the comment docs.

stale, closing. Reopen when there are updates...


fix(image-block): remove useless export

Are you sure it's unused? doing a quick search I can see it in https://github.com/lumapps/design-system/blob/b04e4d75f9fedceae442b4cdca371add12b2e3bf/demo/react/components/lightbox/default.tsx ?

Are you sure it's unused? doing a quick search I can see it in https://github.com/lumapps/design-system/blob/b04e4d75f9fedceae442b4cdca371add12b2e3bf/demo/react/components/lightbox/default.tsx ?

Yep it's not exported anymore since Marco's refactoring. It's now alignment. I will propagate this modification. Thanks for pointing me this issue.


Copyright headers need annual update

Elyra Copyright headers need to be updated to include current year.

Going to need to coordinate this so we dont create too conflicts during merge.
Also copyrights should be updated when the file has been modified.

Also copyrights should be updated when the file has been modified.

Not necessarily, we can just go ahead and update all files in all repos, this is what we have been doing before.

Going to need to coordinate this so we don't create too conflicts during merge.

Git is intelligent enough to make merging easier if there are no changes on the same line.


check for zero cell

Checks if the cell is zero if the given potential is range-separated (i.e. smearing = None) as discussed in #115
Contributor (creator of pull-request) checklist

[x] Tests updated (for new features and bugfixes)?
[ ] Documentation updated (for new features)?
[x] Issue referenced (for PRs that solve an issue)?

Reviewer checklist

[ ] CHANGELOG updated with public API or any other important changes?

📚 Documentation preview 📚: https://torch-pme--131.org.readthedocs.build/en/131/

Also, can I have more permissions to this repo, please? I didn't have enough permissions to create a branch

Also, can I have more permissions to this repo, please? I didn't have enough permissions to create a branch

Sure let me add you to the GH organization.

@PicoCentauri can you check if changelog is updated correctly?


Add memory properties and make it accessible for Vulkan allocation

This pull request exposes the vk::MemoryPropertyFlags of an Allocation via the memory_properties function.
The reason for the request is, that I was not able to obtain the flags on the current release. If it is possible and i just did not find it, please close the request.
I need the flags when checking what the properties of an allocation using the MemoryLocation::CpuToGpu requirement are. Specifically, it is interesting if it contains the DEVICE_LOCAL bit or not. Depending on this code the outcome can be different.
If there is anything you'd like me to add, please let me know!

Yeah, I came across this while trying to use/implement resizable BAR. Checking for all those flags is needed to identify if you need to flush and whether the memory is DeviceLocal.