[MGNLUI-3717] BrowserLocation should treat null 'parameter' argument as an empty string Created: 15/Dec/15  Updated: 24/Dec/15  Resolved: 15/Dec/15

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 5.3.11, 5.4.3
Fix Version/s: 5.3.12, 5.4.4

Type: Bug Priority: Neutral
Reporter: Aleksandr Pchelintcev Assignee: Ngoc Nguyenthanh
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 0.5d
Time Spent: 0.5d
Original Estimate: 1d

Issue Links:
Relates
relates to MGNLUI-3539 Favorites: After opening a bookmark i... Closed
relates to MGNLUI-1026 Update location fragment during conte... Closed
relates to MGNLCE-14 Provide new UI test for favourites Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Saigon 24
Story Points: 1

 Description   

BrowserLocation takes additional argument parameter besides of typical args like app and sub-app. parameter carries information about the view type which should be initially displayed, item path which should be selected etc. All that information is parsed from parameter, but no null check is performed which leads to silly NPE's. Simple fallback to empty string prevents them.



 Comments   
Comment by Ngoc Nguyenthanh [ 22/Dec/15 ]

Here is why it is the cause lead to the issue

  • When user click on a favorite item
  • info.magnolia.ui.contentapp.browser.BrowserSubApp#start will handle the location (in case of Pages app)
  • Then it use info.magnolia.ui.contentapp.browser.BrowserLocation#wrap to create a browser location
  • Unfortunately with the location pattern #app:pages:; location.getParameter() is null
  • Then it can not extractNodePath successful.
  • If you enter directly the link into the browser address, the location will be handled by info.magnolia.ui.vaadin.gwt.client.shared.magnoliashell.Fragment#toString }}(the default is empty string) which used by {{info.magnolia.ui.framework.shell.ShellImpl#goToApp. That why when we enter the address the issue did not happen.
Comment by Mikaël Geljić [ 22/Dec/15 ]

At the moment, I cannot reproduce that standalone. Copying parts of the hipchat discussion here, for the sake of completeness:
First, case above was reproduced on demoauthor, and most likely hit MGNLUI-3539.

  1. Demo doesn't have fix for MGNLUI-3539 (5.4.4); this one most likely fixes the issue as well
  2. Description here claims "silly-NPEs" occur; can we confirm that? or all we have is the preloader hanging? (which would prove MGNLUI-3539, not MGNLUI-3717)
  3. On 5.3.x I cannot produce any NPE either; so it's unclear if the fix is needed at all.
Comment by Mikaël Geljić [ 22/Dec/15 ]

Still no NPE (besides in the unit test), even without fix from MGNLUI-3539, just preloader is just left hanging. Regardless I'll call it done, it doesn't do any harm, and other location fields are null-tolerant as well... Closing. We could bother adding a test for BrowserLocation#wrap but it doesn't do much besides delegating to ctor, so nevermind.

Generated at Mon Feb 12 09:09:24 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.