[review-request][PATCH 0/2] Prevent stale data in ToasterTables

[review-request][PATCH 0/2] Prevent stale data in ToasterTables

[Elliot Smith]

Prevent incorrect caching of ToasterTable data so it doesn't go
stale as the user navigates browser history with back/forward.

Changes since 75bad1b64bd485a50af69ed1f8663a0dadbb5f9e are in:
git://git.yoctoproject.org/poky-contrib, elliot/toaster/bad_caching-7660
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=elliot/toaster/bad_caching-7660

Related bug: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7660

I haven't tested every permutation of table navigation with this patch,
but issue 7660 is fixed.

NB Bug 8294 (https://bugzilla.yoctoproject.org/show_bug.cgi?id=8294)
causes incorrect data to display in a similar way, but the root cause
seems to be different (it's due to incorrect context update there, rather
than caching). This patch doesn't fix that.

Elliot Smith (2):
  toaster: Don't add new history entries when table data loads
  toaster: Don't HTTP cache ToasterTable responses

 bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
 bitbake/lib/toaster/toastergui/widgets.py         | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

--
Elliot Smith
Software Engineer
Intel OTC

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[review-request][PATCH 1/2] toaster: Don't add new history entries when table data loads

[Elliot Smith]

When ToasterTable data is loaded into the UI, a new entry is
added to the browser history. This means that pressing the back
button appears to have no effect, as you end up at the same page,
possibly with slightly different data.

Instead, use replaceState(), so that the browser history doesn't
grow, but the page context still gets updated.

[YOCTO #7660]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
---
 bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bitbake/lib/toaster/toastergui/static/js/table.js b/bitbake/lib/toaster/toastergui/static/js/table.js
index f18034d..99b99a0 100644
--- a/bitbake/lib/toaster/toastergui/static/js/table.js
+++ b/bitbake/lib/toaster/toastergui/static/js/table.js
@@ -49,7 +49,7 @@ function tableInit(ctx){
         headers: { 'X-CSRFToken' : $.cookie('csrftoken')},
         success: function(tableData) {
           updateTable(tableData);
-          window.history.pushState({
+          window.history.replaceState({
               tableData: tableData,
               tableParams: tableParams
           }, null, libtoaster.dumpsUrlParams(tableParams));
-- 
Elliot Smith
Software Engineer
Intel OTC

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[review-request][PATCH 2/2] toaster: Don't HTTP cache ToasterTable responses

[Elliot Smith]

Django allows generated pages to be cached by default by the
browser. This can result in stale data displaying for some pages.

Instead, disable HTTP caching of ToasterTable responses, so that
each time a ToasterTable view is displayed, its data is
refreshed. This carries a performance penalty, but ensures that
ToasterTable views (e.g. compatible layers) are correctly
refreshed if the user navigates their history with forward/back.

[YOCTO #7660]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
---
 bitbake/lib/toaster/toastergui/widgets.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/bitbake/lib/toaster/toastergui/widgets.py b/bitbake/lib/toaster/toastergui/widgets.py
index eb2914d..52ca6be 100644
--- a/bitbake/lib/toaster/toastergui/widgets.py
+++ b/bitbake/lib/toaster/toastergui/widgets.py
@@ -20,6 +20,7 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 from django.views.generic import View, TemplateView
+from django.views.decorators.cache import cache_control
 from django.shortcuts import HttpResponse
 from django.http import HttpResponseBadRequest
 from django.core import serializers
@@ -61,6 +62,10 @@ class ToasterTable(TemplateView):
                         orderable=True,
                         field_name="id")
 
+    # prevent HTTP caching of table data
+    @cache_control(must_revalidate=True, max_age=0, no_store=True, no_cache=True)
+    def dispatch(self, *args, **kwargs):
+        return super(ToasterTable, self).dispatch(*args, **kwargs)
 
     def get(self, request, *args, **kwargs):
         if request.GET.get('format', None) == 'json':
-- 
Elliot Smith
Software Engineer
Intel OTC

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [review-request][PATCH 0/2] Prevent stale data in ToasterTables

[Barros Pena, Belen]

On 14/09/2015 16:09, "toaster-bounces@yoctoproject.org on behalf of Elliot
Smith" <toaster-bounces@yoctoproject.org on behalf of
elliot.smith@intel.com> wrote:

>Prevent incorrect caching of ToasterTable data so it doesn't go
>stale as the user navigates browser history with back/forward.
>
>Changes since 75bad1b64bd485a50af69ed1f8663a0dadbb5f9e are in:
>git://git.yoctoproject.org/poky-contrib, elliot/toaster/bad_caching-7660
>http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=elliot/toaster/ba
>d_caching-7660

This works for me: table entries seem to show the correct states now.

Thanks!

Belén

>
>Related bug: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7660
>
>I haven't tested every permutation of table navigation with this patch,
>but issue 7660 is fixed.
>
>NB Bug 8294 (https://bugzilla.yoctoproject.org/show_bug.cgi?id=8294)
>causes incorrect data to display in a similar way, but the root cause
>seems to be different (it's due to incorrect context update there, rather
>than caching). This patch doesn't fix that.
>
>Elliot Smith (2):
>  toaster: Don't add new history entries when table data loads
>  toaster: Don't HTTP cache ToasterTable responses
>
> bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
> bitbake/lib/toaster/toastergui/widgets.py         | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
>--
>Elliot Smith
>Software Engineer
>Intel OTC
>
>---------------------------------------------------------------------
>Intel Corporation (UK) Limited
>Registered No. 1134945 (England)
>Registered Office: Pipers Way, Swindon SN3 1RJ
>VAT No: 860 2173 47
>
>This e-mail and any attachments may contain confidential material for
>the sole use of the intended recipient(s). Any review or distribution
>by others is strictly prohibited. If you are not the intended
>recipient, please contact the sender and delete all copies.
>
>-- 
>_______________________________________________
>toaster mailing list
>toaster@yoctoproject.org
>https://lists.yoctoproject.org/listinfo/toaster

[PATCH] toaster: layerdetails Fix back button tab behaviour

[Michael Wood]

This completes the behaviour fix of the back button in the layerdetails
page as we not only have parameters in our history we also have the hash
to indicate which tab is active. As we pop our history we need to show
the corresponding tab.

[YOCTO #8252]

Signed-off-by: Michael Wood <michael.g.wood@intel.com>
---
 .../toaster/toastergui/static/js/layerdetails.js   | 28 ++++++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
index 000e803..8817b47 100644
--- a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
+++ b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
@@ -7,6 +7,9 @@ function layerDetailsPageInit (ctx) {
   var layerDepsList = $("#layer-deps-list");
   var currentLayerDepSelection;
   var addRmLayerBtn = $("#add-remove-layer-btn");
+  var targetTab = $("#targets-tab");
+  var machineTab = $("#machines-tab");
+  var detailsTab = $("#details-tab");
 
   /* setup the dependencies typeahead */
   libtoaster.makeTypeahead(layerDepInput, libtoaster.ctx.layersTypeAheadUrl, { include_added: "true" }, function(item){
@@ -15,6 +18,21 @@ function layerDetailsPageInit (ctx) {
     layerDepBtn.removeAttr("disabled");
   });
 
+  $(window).on('hashchange', function(e){
+    switch(window.location.hash){
+      case '#machines':
+        machineTab.tab('show');
+        break;
+      case '#recipes':
+        targetTab.tab('show');
+        break;
+      default:
+        detailsTab.tab('show');
+        break;
+    }
+  });
+
+
   $(".breadcrumb li:first a").click(function(e){
     e.preventDefault();
     /* By default this link goes to the project configuration page. However
@@ -143,7 +161,7 @@ function layerDetailsPageInit (ctx) {
       addRmLayerBtn.removeClass("btn-danger");
   }
 
-  $("#details-tab").on('show', function(){
+  detailsTab.on('show', function(){
     if (!ctx.layerVersion.inCurrentPrj)
       defaultAddBtnText();
 
@@ -174,7 +192,7 @@ function layerDetailsPageInit (ctx) {
       $("#no-recipes-yet").hide();
     }
 
-    $("#targets-tab").removeClass("muted");
+    targetTab.removeClass("muted");
     if (window.location.hash === "#recipes"){
       /* re run the machinesTabShow to update the text */
       targetsTabShow();
@@ -189,7 +207,7 @@ function layerDetailsPageInit (ctx) {
     else
       $("#no-machines-yet").hide();
 
-    $("#machines-tab").removeClass("muted");
+    machineTab.removeClass("muted");
     if (window.location.hash === "#machines"){
       /* re run the machinesTabShow to update the text */
       machinesTabShow();
@@ -202,7 +220,7 @@ function layerDetailsPageInit (ctx) {
 
   });
 
-  $("#targets-tab").on('show', targetsTabShow);
+  targetTab.on('show', targetsTabShow);
 
   function machinesTabShow(){
     if (!ctx.layerVersion.inCurrentPrj) {
@@ -219,7 +237,7 @@ function layerDetailsPageInit (ctx) {
     window.location.hash = "machines";
   }
 
-  $("#machines-tab").on('show', machinesTabShow);
+  machineTab.on('show', machinesTabShow);
 
   $(".pagesize").change(function(){
     var search = libtoaster.parseUrlParams();
-- 
2.1.4

Re: [PATCH] toaster: layerdetails Fix back button tab behaviour

[Brian Avery]

Hi,

I'm confused. If I Navigate as follows, it fails (i think):
start: http://192.168.168.66:8005/toastergui/project/2/
click Recipes, takes me to :
http://192.168.168.66:8005/toastergui/project/2/recipes/?limit=25&page=1&orderby=name&
click Back , takes me to :
http://192.168.168.66:8005/toastergui/project/2/recipes/
click Back again, takes me to:
http://192.168.168.66:8005/toastergui/project/2/
click Forward, takes me to:
http://192.168.168.66:8005/toastergui/project/2/recipes/?limit=25&page=1&orderby=name&
click Back, takes me to:
http://192.168.168.66:8005/toastergui/project/2/recipes/
click Back again, takes me to:
http://192.168.168.66:8005/toastergui/project/2/

---

Seems like 1 forward == two backs.  Is it my browser is chrome on the
mac, though I saw the same behaviour with firefox on ubuntu.

-b

On Wed, Sep 16, 2015 at 4:32 AM, Michael Wood <michael.g.wood@intel.com> wrote:
> This completes the behaviour fix of the back button in the layerdetails
> page as we not only have parameters in our history we also have the hash
> to indicate which tab is active. As we pop our history we need to show
> the corresponding tab.
>
> [YOCTO #8252]
>
> Signed-off-by: Michael Wood <michael.g.wood@intel.com>
> ---
>  .../toaster/toastergui/static/js/layerdetails.js   | 28 ++++++++++++++++++----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> index 000e803..8817b47 100644
> --- a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> +++ b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> @@ -7,6 +7,9 @@ function layerDetailsPageInit (ctx) {
>    var layerDepsList = $("#layer-deps-list");
>    var currentLayerDepSelection;
>    var addRmLayerBtn = $("#add-remove-layer-btn");
> +  var targetTab = $("#targets-tab");
> +  var machineTab = $("#machines-tab");
> +  var detailsTab = $("#details-tab");
>
>    /* setup the dependencies typeahead */
>    libtoaster.makeTypeahead(layerDepInput, libtoaster.ctx.layersTypeAheadUrl, { include_added: "true" }, function(item){
> @@ -15,6 +18,21 @@ function layerDetailsPageInit (ctx) {
>      layerDepBtn.removeAttr("disabled");
>    });
>
> +  $(window).on('hashchange', function(e){
> +    switch(window.location.hash){
> +      case '#machines':
> +        machineTab.tab('show');
> +        break;
> +      case '#recipes':
> +        targetTab.tab('show');
> +        break;
> +      default:
> +        detailsTab.tab('show');
> +        break;
> +    }
> +  });
> +
> +
>    $(".breadcrumb li:first a").click(function(e){
>      e.preventDefault();
>      /* By default this link goes to the project configuration page. However
> @@ -143,7 +161,7 @@ function layerDetailsPageInit (ctx) {
>        addRmLayerBtn.removeClass("btn-danger");
>    }
>
> -  $("#details-tab").on('show', function(){
> +  detailsTab.on('show', function(){
>      if (!ctx.layerVersion.inCurrentPrj)
>        defaultAddBtnText();
>
> @@ -174,7 +192,7 @@ function layerDetailsPageInit (ctx) {
>        $("#no-recipes-yet").hide();
>      }
>
> -    $("#targets-tab").removeClass("muted");
> +    targetTab.removeClass("muted");
>      if (window.location.hash === "#recipes"){
>        /* re run the machinesTabShow to update the text */
>        targetsTabShow();
> @@ -189,7 +207,7 @@ function layerDetailsPageInit (ctx) {
>      else
>        $("#no-machines-yet").hide();
>
> -    $("#machines-tab").removeClass("muted");
> +    machineTab.removeClass("muted");
>      if (window.location.hash === "#machines"){
>        /* re run the machinesTabShow to update the text */
>        machinesTabShow();
> @@ -202,7 +220,7 @@ function layerDetailsPageInit (ctx) {
>
>    });
>
> -  $("#targets-tab").on('show', targetsTabShow);
> +  targetTab.on('show', targetsTabShow);
>
>    function machinesTabShow(){
>      if (!ctx.layerVersion.inCurrentPrj) {
> @@ -219,7 +237,7 @@ function layerDetailsPageInit (ctx) {
>      window.location.hash = "machines";
>    }
>
> -  $("#machines-tab").on('show', machinesTabShow);
> +  machineTab.on('show', machinesTabShow);
>
>    $(".pagesize").change(function(){
>      var search = libtoaster.parseUrlParams();
> --
> 2.1.4
>
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster

Re: [PATCH] toaster: layerdetails Fix back button tab behaviour

[Smith, Elliot]

[-- Attachment #1: Type: text/plain, Size: 5778 bytes --]

Hello Brian.

This is because Toaster does some redirects internally which aren't fixed
by this patch (they would require some substantial refactoring which can't
really be done while the image customisation branch is pending, as the IC
branch changes views.py significantly).

What happens is that Toaster uses RedirectExceptions to control transitions
between some pages, but not pages using ToasterTables. Bug 8252 only refers
to ToasterTable back button behaviour, not the behaviour of these other
pages. I think we need a separate "refactor all pages which don't use
ToasterTable" issue for those; I'll raise it.

Elliot

On 20 September 2015 at 21:27, Brian Avery <avery.brian@gmail.com> wrote:

> Hi,
>
> I'm confused. If I Navigate as follows, it fails (i think):
> start: http://192.168.168.66:8005/toastergui/project/2/
> click Recipes, takes me to :
>
> http://192.168.168.66:8005/toastergui/project/2/recipes/?limit=25&page=1&orderby=name&
> click Back , takes me to :
> http://192.168.168.66:8005/toastergui/project/2/recipes/
> click Back again, takes me to:
> http://192.168.168.66:8005/toastergui/project/2/
> click Forward, takes me to:
>
> http://192.168.168.66:8005/toastergui/project/2/recipes/?limit=25&page=1&orderby=name&
> click Back, takes me to:
> http://192.168.168.66:8005/toastergui/project/2/recipes/
> click Back again, takes me to:
> http://192.168.168.66:8005/toastergui/project/2/
>
> ---
>
> Seems like 1 forward == two backs.  Is it my browser is chrome on the
> mac, though I saw the same behaviour with firefox on ubuntu.
>
> -b
>
> On Wed, Sep 16, 2015 at 4:32 AM, Michael Wood <michael.g.wood@intel.com>
> wrote:
> > This completes the behaviour fix of the back button in the layerdetails
> > page as we not only have parameters in our history we also have the hash
> > to indicate which tab is active. As we pop our history we need to show
> > the corresponding tab.
> >
> > [YOCTO #8252]
> >
> > Signed-off-by: Michael Wood <michael.g.wood@intel.com>
> > ---
> >  .../toaster/toastergui/static/js/layerdetails.js   | 28
> ++++++++++++++++++----
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> > index 000e803..8817b47 100644
> > --- a/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> > +++ b/bitbake/lib/toaster/toastergui/static/js/layerdetails.js
> > @@ -7,6 +7,9 @@ function layerDetailsPageInit (ctx) {
> >    var layerDepsList = $("#layer-deps-list");
> >    var currentLayerDepSelection;
> >    var addRmLayerBtn = $("#add-remove-layer-btn");
> > +  var targetTab = $("#targets-tab");
> > +  var machineTab = $("#machines-tab");
> > +  var detailsTab = $("#details-tab");
> >
> >    /* setup the dependencies typeahead */
> >    libtoaster.makeTypeahead(layerDepInput,
> libtoaster.ctx.layersTypeAheadUrl, { include_added: "true" },
> function(item){
> > @@ -15,6 +18,21 @@ function layerDetailsPageInit (ctx) {
> >      layerDepBtn.removeAttr("disabled");
> >    });
> >
> > +  $(window).on('hashchange', function(e){
> > +    switch(window.location.hash){
> > +      case '#machines':
> > +        machineTab.tab('show');
> > +        break;
> > +      case '#recipes':
> > +        targetTab.tab('show');
> > +        break;
> > +      default:
> > +        detailsTab.tab('show');
> > +        break;
> > +    }
> > +  });
> > +
> > +
> >    $(".breadcrumb li:first a").click(function(e){
> >      e.preventDefault();
> >      /* By default this link goes to the project configuration page.
> However
> > @@ -143,7 +161,7 @@ function layerDetailsPageInit (ctx) {
> >        addRmLayerBtn.removeClass("btn-danger");
> >    }
> >
> > -  $("#details-tab").on('show', function(){
> > +  detailsTab.on('show', function(){
> >      if (!ctx.layerVersion.inCurrentPrj)
> >        defaultAddBtnText();
> >
> > @@ -174,7 +192,7 @@ function layerDetailsPageInit (ctx) {
> >        $("#no-recipes-yet").hide();
> >      }
> >
> > -    $("#targets-tab").removeClass("muted");
> > +    targetTab.removeClass("muted");
> >      if (window.location.hash === "#recipes"){
> >        /* re run the machinesTabShow to update the text */
> >        targetsTabShow();
> > @@ -189,7 +207,7 @@ function layerDetailsPageInit (ctx) {
> >      else
> >        $("#no-machines-yet").hide();
> >
> > -    $("#machines-tab").removeClass("muted");
> > +    machineTab.removeClass("muted");
> >      if (window.location.hash === "#machines"){
> >        /* re run the machinesTabShow to update the text */
> >        machinesTabShow();
> > @@ -202,7 +220,7 @@ function layerDetailsPageInit (ctx) {
> >
> >    });
> >
> > -  $("#targets-tab").on('show', targetsTabShow);
> > +  targetTab.on('show', targetsTabShow);
> >
> >    function machinesTabShow(){
> >      if (!ctx.layerVersion.inCurrentPrj) {
> > @@ -219,7 +237,7 @@ function layerDetailsPageInit (ctx) {
> >      window.location.hash = "machines";
> >    }
> >
> > -  $("#machines-tab").on('show', machinesTabShow);
> > +  machineTab.on('show', machinesTabShow);
> >
> >    $(".pagesize").change(function(){
> >      var search = libtoaster.parseUrlParams();
> > --
> > 2.1.4
> >
> > --
> > _______________________________________________
> > toaster mailing list
> > toaster@yoctoproject.org
> > https://lists.yoctoproject.org/listinfo/toaster
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>



-- 
Elliot Smith
Software Engineer
Intel Open Source Technology Centre

[-- Attachment #2: Type: text/html, Size: 8543 bytes --]

Re: [PATCH] toaster: layerdetails Fix back button tab behaviour

[Smith, Elliot]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

On 21 September 2015 at 09:35, Smith, Elliot <elliot.smith@intel.com> wrote:

> This is because Toaster does some redirects internally which aren't fixed
> by this patch (they would require some substantial refactoring which can't
> really be done while the image customisation branch is pending, as the IC
> branch changes views.py significantly).
>
> What happens is that Toaster uses RedirectExceptions to control
> transitions between some pages, but not pages using ToasterTables. Bug 8252
> only refers to ToasterTable back button behaviour, not the behaviour of
> these other pages. I think we need a separate "refactor all pages which
> don't use ToasterTable" issue for those; I'll raise it.
>

I raised this bug to cover the initial investigation of what needs to be
done:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=8363

Elliot
-- 
Elliot Smith
Software Engineer
Intel Open Source Technology Centre

[-- Attachment #2: Type: text/html, Size: 1539 bytes --]

Re: [PATCH] toaster: layerdetails Fix back button tab behaviour

[Brian Avery]

k.
ty, I'll upstream this one this morning then.
-b

On Mon, Sep 21, 2015 at 1:43 AM, Smith, Elliot <elliot.smith@intel.com> wrote:
> On 21 September 2015 at 09:35, Smith, Elliot <elliot.smith@intel.com> wrote:
>>
>> This is because Toaster does some redirects internally which aren't fixed
>> by this patch (they would require some substantial refactoring which can't
>> really be done while the image customisation branch is pending, as the IC
>> branch changes views.py significantly).
>>
>> What happens is that Toaster uses RedirectExceptions to control
>> transitions between some pages, but not pages using ToasterTables. Bug 8252
>> only refers to ToasterTable back button behaviour, not the behaviour of
>> these other pages. I think we need a separate "refactor all pages which
>> don't use ToasterTable" issue for those; I'll raise it.
>
>
> I raised this bug to cover the initial investigation of what needs to be
> done:
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=8363
>
> Elliot
> --
> Elliot Smith
> Software Engineer
> Intel Open Source Technology Centre

Re: [PATCH] toaster: layerdetails Fix back button tab behaviour

[Brian Avery]

k. so not upstreamed in the morning but upstreamed all the same :).
ty,
b

On Mon, Sep 21, 2015 at 7:41 AM, Brian Avery <avery.brian@gmail.com> wrote:
> k.
> ty, I'll upstream this one this morning then.
> -b
>
> On Mon, Sep 21, 2015 at 1:43 AM, Smith, Elliot <elliot.smith@intel.com> wrote:
>> On 21 September 2015 at 09:35, Smith, Elliot <elliot.smith@intel.com> wrote:
>>>
>>> This is because Toaster does some redirects internally which aren't fixed
>>> by this patch (they would require some substantial refactoring which can't
>>> really be done while the image customisation branch is pending, as the IC
>>> branch changes views.py significantly).
>>>
>>> What happens is that Toaster uses RedirectExceptions to control
>>> transitions between some pages, but not pages using ToasterTables. Bug 8252
>>> only refers to ToasterTable back button behaviour, not the behaviour of
>>> these other pages. I think we need a separate "refactor all pages which
>>> don't use ToasterTable" issue for those; I'll raise it.
>>
>>
>> I raised this bug to cover the initial investigation of what needs to be
>> done:
>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=8363
>>
>> Elliot
>> --
>> Elliot Smith
>> Software Engineer
>> Intel Open Source Technology Centre

Re: [review-request][PATCH 1/2] toaster: Don't add new history entries when table data loads

[Michael Wood]

On 14/09/15 16:09, Elliot Smith wrote:
> When ToasterTable data is loaded into the UI, a new entry is
> added to the browser history. This means that pressing the back
> button appears to have no effect, as you end up at the same page,
> possibly with slightly different data.
>
> Instead, use replaceState(), so that the browser history doesn't
> grow, but the page context still gets updated.
>
> [YOCTO #7660]
>
> Signed-off-by: Elliot Smith <elliot.smith@intel.com>
> ---
>   bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/bitbake/lib/toaster/toastergui/static/js/table.js b/bitbake/lib/toaster/toastergui/static/js/table.js
> index f18034d..99b99a0 100644
> --- a/bitbake/lib/toaster/toastergui/static/js/table.js
> +++ b/bitbake/lib/toaster/toastergui/static/js/table.js
> @@ -49,7 +49,7 @@ function tableInit(ctx){
>           headers: { 'X-CSRFToken' : $.cookie('csrftoken')},
>           success: function(tableData) {
>             updateTable(tableData);
> -          window.history.pushState({
> +          window.history.replaceState({
>                 tableData: tableData,
>                 tableParams: tableParams
>             }, null, libtoaster.dumpsUrlParams(tableParams));

If we replace state each time we load, we won't have a back stack to put 
the tableData into and subsequently pop it out of on the onpopstate 
event, could you also remove storing the tableData, tableParams in the 
browser and remove the window.onpopstate handler code as that won't ever 
fire now.

Thanks,

Michael

Re: [review-request][PATCH 1/2] toaster: Don't add new history entries when table data loads

[Smith, Elliot]

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]

On 29 September 2015 at 18:50, Michael Wood <michael.g.wood@intel.com>
wrote:

> On 14/09/15 16:09, Elliot Smith wrote:
>
>> When ToasterTable data is loaded into the UI, a new entry is
>> added to the browser history. This means that pressing the back
>> button appears to have no effect, as you end up at the same page,
>> possibly with slightly different data.
>>
>> Instead, use replaceState(), so that the browser history doesn't
>> grow, but the page context still gets updated.
>>
>> [YOCTO #7660]
>>
>> Signed-off-by: Elliot Smith <elliot.smith@intel.com>
>> ---
>>   bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/bitbake/lib/toaster/toastergui/static/js/table.js
>> b/bitbake/lib/toaster/toastergui/static/js/table.js
>> index f18034d..99b99a0 100644
>> --- a/bitbake/lib/toaster/toastergui/static/js/table.js
>> +++ b/bitbake/lib/toaster/toastergui/static/js/table.js
>> @@ -49,7 +49,7 @@ function tableInit(ctx){
>>           headers: { 'X-CSRFToken' : $.cookie('csrftoken')},
>>           success: function(tableData) {
>>             updateTable(tableData);
>> -          window.history.pushState({
>> +          window.history.replaceState({
>>                 tableData: tableData,
>>                 tableParams: tableParams
>>             }, null, libtoaster.dumpsUrlParams(tableParams));
>>
>
> If we replace state each time we load, we won't have a back stack to put
> the tableData into and subsequently pop it out of on the onpopstate event,
> could you also remove storing the tableData, tableParams in the browser and
> remove the window.onpopstate handler code as that won't ever fire now.
>

Thanks for the review.

I've added a commit which (I think) removes the code you're talking about.
It's 0ea00338f688d4b90ab5794cd2bbb181db880d10 on
the elliot/toaster/bad_caching-7660 branch. Could you please verify that
I've understood your comment correctly? If I have, I'll resubmit the
patches.

Thanks.
Elliot
-- 
Elliot Smith
Software Engineer
Intel Open Source Technology Centre

[-- Attachment #2: Type: text/html, Size: 2995 bytes --]

Re: [review-request][PATCH 1/2] toaster: Don't add new history entries when table data loads

[Michael Wood]

On 30/09/15 10:00, Smith, Elliot wrote:
> On 29 September 2015 at 18:50, Michael Wood <michael.g.wood@intel.com 
> <mailto:michael.g.wood@intel.com>> wrote:
>
>     On 14/09/15 16:09, Elliot Smith wrote:
>
>         When ToasterTable data is loaded into the UI, a new entry is
>         added to the browser history. This means that pressing the back
>         button appears to have no effect, as you end up at the same page,
>         possibly with slightly different data.
>
>         Instead, use replaceState(), so that the browser history doesn't
>         grow, but the page context still gets updated.
>
>         [YOCTO #7660]
>
>         Signed-off-by: Elliot Smith <elliot.smith@intel.com
>         <mailto:elliot.smith@intel.com>>
>         ---
>           bitbake/lib/toaster/toastergui/static/js/table.js | 2 +-
>           1 file changed, 1 insertion(+), 1 deletion(-)
>
>         diff --git a/bitbake/lib/toaster/toastergui/static/js/table.js
>         b/bitbake/lib/toaster/toastergui/static/js/table.js
>         index f18034d..99b99a0 100644
>         --- a/bitbake/lib/toaster/toastergui/static/js/table.js
>         +++ b/bitbake/lib/toaster/toastergui/static/js/table.js
>         @@ -49,7 +49,7 @@ function tableInit(ctx){
>                   headers: { 'X-CSRFToken' : $.cookie('csrftoken')},
>                   success: function(tableData) {
>                     updateTable(tableData);
>         -          window.history.pushState({
>         +          window.history.replaceState({
>                         tableData: tableData,
>                         tableParams: tableParams
>                     }, null, libtoaster.dumpsUrlParams(tableParams));
>
>
>     If we replace state each time we load, we won't have a back stack
>     to put the tableData into and subsequently pop it out of on the
>     onpopstate event, could you also remove storing the tableData,
>     tableParams in the browser and remove the window.onpopstate
>     handler code as that won't ever fire now.
>
>
> Thanks for the review.
>
> I've added a commit which (I think) removes the code you're talking 
> about. It's 0ea00338f688d4b90ab5794cd2bbb181db880d10 on 
> the elliot/toaster/bad_caching-7660 branch. Could you please verify 
> that I've understood your comment correctly? If I have, I'll resubmit 
> the patches.
>
> Thanks.
> Elliot
> -- 
> Elliot Smith
> Software Engineer
> Intel Open Source Technology Centre
>
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>

Thanks.

Patches submitted upstream, with a minor fix to keeping the update of 
the url parameters in the replaceState.

Michael