1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Access Restrictions GUI bug with Firefox 23 - can't delete rules

Discussion in 'Tomato Firmware' started by koitsu, Aug 20, 2013.

  1. koitsu

    koitsu Network Guru Member

    It appears that in firmware tomato-K26USB-1.28.0502.8MIPSR2Toastman-RT-N-Ext.trx I cannot delete Access Restrictions entries when using Firefox 23. Reproduction behaviour:

    * Add a new Access Restrictions rule. Attributes/conditionals don't matter
    * Save the new rule, and (as applicable) will be enabled/put into place
    * Go back to Access Restrictions and you'll see the rule there
    * Click on the rule you added
    * Click on the "Delete..." button
    * The button disappears and no change happens behind the scenes
    * I also tried clicking "Save" at this point -- also has no effect

    What SHOULD be happening here is a pop-up dialog box titled "Message from webpage" that says "Delete this rule?" with OK/Cancel options.

    As a test, I disabled Firefox's "Block pop-up windows" feature -- no difference.

    If I use Internet Explorer 8 I am able to delete the rule (the pop-up dialog box appears and the rule does get deleted when clicking OK).

    This is pretty major if you ask me, and indicates that someone familiar with Javascript/etc. will need to hash out what is going on behind the scenes. Firefox obviously does not take kindly to however this is implemented (or it's a regression in Firefox). The easiest workaround is just to make the "Delete" button always delete the rule -- no confirmation box, just do what the user clicked. If someone accidentally deletes a rule when they meant to click save, well IMHO, tough titty -- people should be paying attention to what they're doing when messing around in access areas like this.

    I myself am not a Javascript guru, so if folks who are have some ideas, I'm happy to bang away on things.
     
  2. koitsu

    koitsu Network Guru Member

  3. mstombs

    mstombs Network Guru Member

    I can also confirm this bug affects Shibby

    Tomato Firmware 1.28.0000 MIPSR2-110 K26 USB AIO-64K on an RT-N66U

    using Firefox 23 and Chrome 28 running on Ubuntu 12.04

    But can delete the rule using Android 4.1.2 Browser on mobile

    Can also use Opera 12 successfully on Ubuntu PC.

    I wonder if the name of the javascript button function "remove()" is the problem?

    I am pretty sure it is, in Chrome tools you can add a function in the console, then change the attribute of the button

    Code:
    function myremove()
    {
    if (!confirm('Delete this rule?')) return;
    E('delete-button').disabled = 1;
    e = E('_rrule');
    e.name = 'rrule' + rruleN;
    e.value = '';
    form.submit('_fom');
    }
    Then it works...

    [Confirmation]

    http://stackoverflow.com/questions/16151295/is-remove-a-reserved-keyword-in-google-chrome

    So an alternative is to rename the action of the button to "window.remove()"
     
    Last edited: Aug 21, 2013
  4. Monk E. Boy

    Monk E. Boy Network Guru Member

    Creating a bug on Mozilla's end (bugzilla) would probably be helpful, since it was a change on their end that broke things.

    Edit: If I'm relatively conscious after returning home tonight I'll see about creating a dummy email address, registering there, then opening a bug on this.
     
    Last edited: Aug 22, 2013
  5. eahm

    eahm LI Guru Member

  6. koitsu

    koitsu Network Guru Member

    I don't think that's necessarily the case, because this is affecting at least 2 separate browsers who use completely different cores (Firefox = Gecko, Chrome=WebKit). Likewise, it would be good if folks could test with Opera, IE9, IE10, and Konqueror (if anyone using *IX with X is around).

    This is almost certainly a result of someone designing some Javascript bits in Tomato "that worked" when they wrote it, but wasn't designed/done correctly. mstombs' statement that the user-defined function is called remove() which conflicts with the built-in function name, to me, is not "a browser bug" -- that's a developer (of the JS code) who made a very, very bad choice when naming their functions.

    So simply put: changing the function name from remove() to something like myremove() is the correct solution. I'll see if I can come up with a diff, time permitted.
     
  7. Monk E. Boy

    Monk E. Boy Network Guru Member

    My point is that this behavior is new in Firefox, since the function worked previously, so it represents a change in Firefox.

    While this particular function in Chrome hasn't worked for quite some time, if it ever worked, as someone who has to provide support Chrome for a living I can assure you that websites being broken in Chrome is a far from an unusual occurrence.

    Personally I would rather see Firefox correct their new behavior than change the website, because there's a lot of people floating around with old versions. Ideally the function should have been named correctly in the first place, but we've got to live with it. :(
     
    Last edited: Aug 22, 2013
  8. koitsu

    koitsu Network Guru Member

    But we don't have to live with it, that's my point -- the function can (and should) be renamed in Tomato. Some other bits/pieces of the Tomato JavaScript code have already been changed recently as well. So as I see it, the only limiting factor is me making time to come up with a proper diff.
     
  9. koitsu

    koitsu Network Guru Member

    And here's the diff, against Toastman-RT-N branch. Note that I've renamed the remove(), save(), and cancel() functions to removeRule(), saveRule(), and cancelRule() respectively. I did this because I wasn't going to take any chances. :) And naturally I updated the HTML that refers to their names.

    Included both an attachment as well as the raw diff below.

    Code:
    diff --git a/release/src/router/www/restrict-edit.asp b/release/src/router/www/restrict-edit.asp
    index 00cbcd3..b5a1eea 100644
    --- a/release/src/router/www/restrict-edit.asp
    +++ b/release/src/router/www/restrict-edit.asp
    @@ -309,12 +309,12 @@ function verifyFields(focused, quiet)
      return 1;
     }
    
    -function cancel()
    +function cancelRule()
     {
      document.location = 'restrict.asp';
     }
    
    -function remove()
    +function removeRule()
     {
      if (!confirm('Delete this rule?')) return;
    
    @@ -326,7 +326,7 @@ function remove()
      form.submit('_fom');
     }
    
    -function save()
    +function saveRule()
     {
      if (!verifyFields(null, false)) return;
      if ((cg.isEditing()) || (bpg.isEditing())) return;
    @@ -495,10 +495,10 @@ createFieldTable('', [
     </td></tr>
     <tr><td id='footer' colspan=2>
      <span id='footer-msg'></span>
    -  <input type='button' value='Delete...' id='delete-button' onclick='remove()'>
    +  <input type='button' value='Delete...' id='delete-button' onclick='removeRule()'>
      &nbsp;
    -  <input type='button' value='Save' id='save-button' onclick='save()'>
    -  <input type='button' value='Cancel' id='cancel-button' onclick='cancel()'>
    +  <input type='button' value='Save' id='save-button' onclick='saveRule()'>
    +  <input type='button' value='Cancel' id='cancel-button' onclick='cancelRule()'>
     </td></tr>
     </table>
     <br><br>
    
     

    Attached Files:

    Toastman, Victek, jerrm and 1 other person like this.
  10. Elfew

    Elfew Addicted to LI Member

    Same problem with Victek build
     
  11. Victek

    Victek Network Guru Member

  12. nmalinoski

    nmalinoski Serious Server Member

    I experienced similar behavior in Toastman 1.28.7634 for the WRT54G/GL (IPT-ND-VLAN-VPN) when trying to delete items from the Virtual Wireless menu; it wouldn't let me delete any virtual networks from the list.
     
  13. Elfew

    Elfew Addicted to LI Member

    But it is normal because there is no command for deleting vlan... You can just disable it.
     
  14. nmalinoski

    nmalinoski Serious Server Member

    Virtual Wireless networks, not VLANs; different menu.
     
  15. Elfew

    Elfew Addicted to LI Member

    Srry I mean virtual wireless network - just use search ;)
     
  16. koitsu

    koitsu Network Guru Member

    I don't understand what's been said in the past 4 posts.

    This thread is about Access Restrictions. If there are similar problems with the buttons disappearing/acting as no-ops in sections/feature areas other than Access Restrictions, then please start a separate thread about that.
     

Share This Page