Project

General

Profile

Bug #5488

services_dnsmasq.php - Clicking "apply" causes a config save and removes settings

Added by Jim Pingle over 3 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Very High
Assignee:
Category:
DNS Forwarder
Target version:
Start date:
11/19/2015
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.3
Affected Architecture:
All

Description

On services_dnsmasq.php if you edit a host override and then save, you are returned to services_dnsmasq.php and an "Apply" button is offered.

Clicking "Apply" here submits the page and somehow causes the config to lose any settings directly on the page, including the Enable option, DHCP host registration options, custom options, etc.

The apply action shouldn't be triggering an additional save, the other fields were probably not submitted.

Associated revisions

History

#1 Updated by Steve Beaver over 3 years ago

  • Status changed from New to Feedback
  • Assignee changed from Steve Beaver to Jim Pingle

Oddly there is no code (2.2.x or 2.3) to process the "Apply" button. In 2.2 this does no harm, in 2.3 it did.

For now I have made a work-around in that clicking the "Apply" button displays a banner advising the user to reboot the firewall for the changes to take effect. THis is at lease slightly better than 2.2.x :)

#2 Updated by Jim Pingle over 3 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from Jim Pingle to Steve Beaver

It doesn't/shouldn't require a reboot to activate the settings though. If you actually press save on the main screen instead of Apply, the override entries get updated successfully.

Looks like the Apply action should run this part:

            // Reload filter (we might need to sync to CARP hosts)
            filter_configure();
            /* Update resolv.conf in case the interface bindings exclude localhost. */
            system_resolvconf_generate();
            /* Start or restart dhcpleases when it's necessary */
            system_dhcpleases_configure();
            if ($retval == 0) {
                clear_subsystem_dirty('hosts');
            }

#3 Updated by Phillip Davis over 3 years ago

I just checked on 2.2.5. After adding a host override and before pressing Apply the new host name is not known. After pressing Apply it is known. I guess the code in 2.2.5, when Apply is pressed, sees a whole $_POST of the main page and processes that? It certainly implements the pending new host entry.

#4 Updated by Steve Beaver over 3 years ago

That's right. The problem is that in 2.2 the "Alert" button lives inside the main form so submits every field. In 2.3 it lives in its own little form so submits only itself. I have a plan to fix it, however. I should get to it tomorrow.

#5 Updated by Phillip Davis over 3 years ago

Note that when adding/editing a domain override services_dnsmasq_domainoverride_edit.php applies the changes itself.

$retval = services_dnsmasq_configure();

and thus there is no Apply button needed back on the main page.

I wonder why this inconsistency in implementation?

Maybe it could be made consistent? If services_dnsmasq_edit.php did the same sort of thing when saving the host override then there is no need to mark_subsystem_dirty and thus no need for the Apply button.

A downside of auto-apply with every change is that if you are making lots of add/edits then it can be nice to do a single Apply at the end and avoid lots of little interruptions to DNS service for users.

#6 Updated by Steve Beaver over 3 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from Steve Beaver to Jim Pingle

The "Apply" button is now updated via jQuery to submit the main form (instead of the form created by the alert box) thereby restoring the original behavior.

#7 Updated by Steve Beaver over 3 years ago

  • % Done changed from 0 to 100

#8 Updated by Jim Pingle over 3 years ago

  • Status changed from Feedback to Resolved

This works OK now -- we had some discussion in chat last night about making it consistently ask for apply everywhere though. I'll open a separate ticket for that.

Also available in: Atom PDF