Change Details

Currently Parsoid/PHP crashes if there's not a valid srcText specified in the top-level Frame (which is `PageConfigFrame`). This is used in both serialize and parse (see T234548 -- technically we use SelserData instead of Frame right now). We sort of kludge around this by using the empty string `''` as a srcText when we create the PageConfig/Env in a number of paths, but this masks cases where this could be an actual bug (ie, where srcText is required but we're not providing it). One solution might be to add a boolean parameter to the PageConfigFactory (or something like that) which would come all the way from the API and indicate when we expect this to be a new page (in which case we won't have a srcText) vs where we expect there to be a previous revision and we should error out if we can't fetch the text of that.

Currently Parsoid/PHP crashes if there's not a valid srcText specified in the top-level Frame (which is `PageConfigFrame`). This is used in both serialize and parse (see T234548 -- technically we use SelserData instead of Frame right now). We sort of kludge around this by using the empty string `''` as a srcText when we create the PageConfig/Env in a number of paths, but this masks cases where this could be an actual bug (ie, where srcText is required but we're not providing it). One solution might be to add a boolean parameter to the PageConfigFactory (or something like that) which would come all the way from the API and indicate when we expect this to be a new page (in which case we won't have a srcText) vs where we expect there to be a previous revision and we should error out if we can't fetch the text of that. UPDATE 2020-08-24: {T259855} showed a related version of this bug: the REST API itself doesn't clearly distinguish between "new page creation" (where revid is omitted because there is none yet) and "edit of existing page" (where revid should not be omitted, on pain of corruption). Core has recently introduced `RevisionRecord` to address this to some degree: even unsaved pages have a `RevisionRecord`, although they may not yet have a revisionid. So in addition to clearly distinguishing between new-page-creation and editing-existing-page in the Frame API, we should also address this at the REST API and ParserCache/RestBase level in order to catch potential corruption bugs and avoid issues where "missing revision id" or "failed fetch of revision id" becomes a page-corruption bug by falling back into the "new page creation" codepaths.

Currently Parsoid/PHP crashes if there's not a valid srcText specified in the top-level Frame (which is `PageConfigFrame`). This is used in both serialize and parse (see T234548 -- technically we use SelserData instead of Frame right now). We sort of kludge around this by using the empty string `''` as a srcText when we create the PageConfig/Env in a number of paths, but this masks cases where this could be an actual bug (ie, where srcText is required but we're not providing it). One solution might be to add a boolean parameter to the PageConfigFactory (or something like that) which would come all the way from the API and indicate when we expect this to be a new page (in which case we won't have a srcText) vs where we expect there to be a previous revision and we should error out if we can't fetch the text of that. UPDATE 2020-08-24: {T259855} showed a related version of this bug: the REST API itself doesn't clearly distinguish between "new page creation" (where revid is omitted because there is none yet) and "edit of existing page" (where revid should not be omitted, on pain of corruption). Core has recently introduced `RevisionRecord` to address this to some degree: even unsaved pages have a `RevisionRecord`, although they may not yet have a revisionid. So in addition to clearly distinguishing between new-page-creation and editing-existing-page in the Frame API, we should also address this at the REST API and ParserCache/RestBase level in order to catch potential corruption bugs and avoid issues where "missing revision id" or "failed fetch of revision id" becomes a page-corruption bug by falling back into the "new page creation" codepaths.