StackOverflowMATLABchat / mlapptools

MATLAB class containing methods for programmatic uifigure modification
http://undocumentedmatlab.com/blog/customizing-uifigures-part-2
MIT License
25 stars 11 forks source link

while/try/catch block for UIFigure presence too inclusive #1

Closed sco1 closed 6 years ago

sco1 commented 8 years ago

The while/try/catch block used to wait for the existence of the parent UIFigure seems overly inclusive and the condition for exiting the block can lead to false positives.

e.g.

rez = '';
while ~strcmp(rez, sprintf('"%s"', alignment))
    try
        % Get a handle to the webwindow
        win = mlapptools.getwebwindow(uielement.Parent);

        % Find which element of the DOM we want to edit
        data_tag = mlapptools.getdatatag(uielement);

        % Manipulate the DOM via a JS command
        widgetID = mlapptools.getwidgetID(win, data_tag);

        alignsetstr = sprintf('dojo.style(dojo.query("#%s")[0], "textAlign", "%s")', widgetID, alignment);
        rez = win.executeJS(alignsetstr);
    catch err
        pause(1); % Give the figure (webpage) some more time to load
    end
end

There is likely a better way to query and wait for the existence of the figure window, which can be shifted into a generic helper method.

The while block can also lead to false positives, such as the situation resolved in cb9c465. Though we were operating on the incorrect DOM, textAlign would execute without error/warning because we're setting a "meaningless" property which is enough to satisfy the while criteria.

Dev-iL commented 8 years ago

We could respond to a "page finished" callback that is issued by the CEF (browser) asynchronously from MATLAB's main thread (I didn't know MATLAB could handle that tbh). So in our app code we define a flag that tracks the loading state:

    properties (GetAccess = public, SetAccess = private)
      didPageFinishLoading = false;
    end

Next we define a private function that can set this value to true:

        function confirmLoadingFinished(app,varargin)
          app.didPageFinishLoading = true;
        end

Then we set the above function as our MATLAB-side callback using:

win.PageLoadFinishedCallback = @app.confirmLoadingFinished;

And we get:

function startupFcn(app)
  TIMEOUT = 10; tic;
  while true && (toc < TIMEOUT)
    try   
      win = struct(struct(struct(app).UIFigure).Controller).Container.CEF;
      win.PageLoadFinishedCallback = @app.confirmLoadingFinished;
      break;
    catch ex
      disp(['Not ready yet! Error: ' ex.message]);
      pause(0.5); % Give the figure (webpage) some more time to load, but not too much so we 
                  %   don't miss the opportunity to set the callback.
    end
  end
  TIMEOUT = 10; tic; %[sec], safety measure for missing the callback.
  while ~app.didPageFinishLoading && (toc < TIMEOUT)
    pause(0.1);
  end
  if app.didPageFinishLoading
    pause(0.1); % Can't explain why, but this is required. Might not be enough.
    applyStyles(app, win); % this is when all data_tag / widgetId / dojo things happen
  else
    warning('Timeout reached! Styles not applied!');
  end
end

P.S. 1 Instead of timeout "as time" one can define a timeout as a number of iterations:

N_ITER_BEFORE_TIMEOUT = 100;
for ind = 1:N_ITER_BEFORE_TIMEOUT
    assert(ind <= N_ITER_BEFORE_TIMEOUT,'Timeout reached!');
    try % Get a handle to the webwindow
    ... % (same as before)
    end
end

The strcmp(rez, sprintf('"%s"', alignment)) test can be moved to the end of the function to report if the manipulation [seems to have] worked.

P.S. 2 If you want syntax highlighting in your code blocks, start them with "matlab" instead of just ""

sco1 commented 8 years ago

I definitely like the idea of the callback, but you end up still needing at least one of the try/catch blocks and you also have the additional requirement of adding a property and method to every app we'd want to have robust handling in.

I feel like there has to be a way to query this directly from the DOM, given the existence of dojo/ready and dojo/domReady! the capability clearly exists, I just have no idea how to get to it from MATLAB.


There are 3 conditions that we seem to be racing:

MATLAB:nonExistentField
Reference to non-existent field 'Container'.

Once the UIFigure window is completely rendered, figurewindow.Controller becomes non-empty so we can obtain the webwindow object from figurewindow.Controller.Container.CEF. This webwindow object handle is needed for the above loading confirmation callback.

cefclient:webwindow:jserror
Error executing JavaScript command: 

    JavaScript error: Uncaught ReferenceError: dojo is not defined at line 1 column 0 in undefined

Once we go to obtain the widget ID, we're racing Dojo's initialization

cefclient:webwindow:jserror
Error executing JavaScript command: 

    JavaScript error: Uncaught TypeError: Cannot read property 'widgetid' of null at line 152 column 68 in http://localhost:31415/toolbox/matlab/uitools/uifigureappjs/release/dojo/dojo.js

Once we get past Dojo initializing, now we're racing the initialization of the web element itself


The testing code:

% Create UIFigure
app.UIFigure = uifigure;
app.UIFigure.Position = [100 100 260 147];
app.UIFigure.Name = 'UI Figure';

% Create ListBox
app.ListBox = uilistbox(app.UIFigure);
app.ListBox.Position = [109 36 100 74];

warning off MATLAB:HandleGraphics:ObsoletedProperty:JavaFrame
warning off MATLAB:structOnObject

errstrs = {};
while true
    try
        win = struct(struct(app.UIFigure).Controller).Container.CEF;

        data_tag = char(struct(app.ListBox).Controller.ProxyView.PeerNode.getId);

        widgetquerystr = sprintf('dojo.getAttr(dojo.query("[data-tag^=''%s''] > div")[0], "widgetid")', data_tag);
        widgetID = win.executeJS(widgetquerystr);
        widgetID = widgetID(2:end-1);

        alignsetstr = sprintf('dojo.style(dojo.query("#%s")[0], "textAlign", "%s")', widgetID, 'center');
        win.executeJS(alignsetstr);
        break
    catch err
        if ~any(ismember(errstrs, err.message));
            errstrs{end + 1} = err.message;
            fprintf('%s\n%s\n\n', err.identifier, err.message);
        end
        pause(0.01)
    end
end

warning on MATLAB:HandleGraphics:ObsoletedProperty:JavaFrame
warning on MATLAB:structOnObject
Dev-iL commented 8 years ago

@sco1 I'd like to mention that I initially tried including a call to dojo.version while waiting for win to become available, however this didn't work, as the availability of dojo happens before the elements are completely initialized, thus giving the "null element" error you mentioned.

It seems you found something better for checking the readiness of the DOM. If however, your suggestion has the same effect, I can't think of anything other than the "callback + pause" combination, or try to query some element in a loop until it succeeds.

sco1 commented 6 years ago

Should be fixed with #8 (947c54e)